Skip to content
This repository has been archived by the owner on Apr 2, 2019. It is now read-only.

Column attribute support & Header row sort support #501

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Column attribute support & Header row sort support #501

wants to merge 1 commit into from

Conversation

WRidder
Copy link

@WRidder WRidder commented Jun 11, 2014

To add support for the following extensions; Reorderable columns, Grouped columns and (re)sizeable columns I propose a few small changes to backgrid.

These are:

  • 'attributes' support on column definition. Maybe we should limit this using pick to allow only valid html5 attributes. This is one way to support 'colspan' and 'rowspan' on columns.
  • Header rows did not update after a sort, now they do

Added support for the following extensions: Reorderable columns, Grouped columns and (re)sizeable columns.
@wyuenho
Copy link
Contributor

wyuenho commented Jun 12, 2014

There are many issues and concerns with this PR.

  1. Style. Please read the CONTRIBUTING.md
  2. Please utility underscore.js and stop using ES5 only constructs. We still support IE8 at the moment.
  3. The assumption in Backgrid is there is a single column for each model attribute. You are allowing the header cell to span but not the regular table cells.
  4. Why don't you support colgroups and cols? It's much easier to style them than individual header cells.
  5. What happens if you toggle renderable/sortable/editable?

It would be nice if you could copy and paste all your documentations and repo URLs here, and reference all the relevant tickets so we can concentrate the discussion in one place.

Thanks!

@wyuenho
Copy link
Contributor

wyuenho commented Jun 13, 2014

BTW, please don't edit lib/backgrid.js directly. It's automatically generated from the files under src

@WRidder
Copy link
Author

WRidder commented Jun 20, 2014

1 & 2:
I've updated the repository here: https://github.com/WRidder/backgrid/tree/backgrid-extension-support. I removed the ES5-only constructs, applied changes to the src files and made sure it works with IE8.

3:
That is correct. I'm working keeping certain use cases in mind of course and I don't have any personally which requires column cells to span several columns. We can look into this of course. I suppose that one of usages can be grouped sections.

Can you elaborate on this? The extensions (especially the sizeable columns) do use col and colgroup elements.

Sortable shouldn't be influenced and is currently working correctly as far as I can see. However, you can only sort the base columns (headercells which directly correlate to a grid column) as of now. I'm not sure about parent column sort options. I've conducted no tests yet with editable, however, I don't expect there to be much problems as of right now. Regarding renderable; that does exhibit still some issues I've yet to adress.

Demo

http://techwuppet.com/backgrid_poc_demo/

Repos

https://github.com/WRidder/backgrid/tree/backgrid-extension-support
https://github.com/WRidder/backgrid-grouped-columns/
https://github.com/WRidder/backgrid-sizeable-columns
https://github.com/WRidder/backgrid-orderable-columns

Related backgrid issues

#490
#392
#335

@WRidder WRidder mentioned this pull request Jun 26, 2014
@WRidder
Copy link
Author

WRidder commented Jul 9, 2014

Anyone comments on the current progress? Currently I am looking into adding tests to the plugins; however, first I would like to have a consensus regarding the plugin design and Backgrid modifications.

@excentris
Copy link

@wyuenho Could you comment on the current progress as described by @WRidder on his comments?
Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants