Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Issue #312 - added getNumColumns(), setNumColumns(value) for grid row interface #703

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants
  1. Grid row interface: getNumColumns(), setNumColumns(value)
  2. For setNumColumns(value), not sure if I should check if value is a non-negative integer and throw an error if not.
Member

tneil commented Feb 18, 2013

I'll try and review this pull request tomorrow

Member

tneil commented Feb 19, 2013

When you set the number of columns in a row, the row must then recompute and layout based on the new number of columns. The current code in your pull request for setNumColumns() only updates the attribute for the row.

To accomplish this, I think the grid code needs to be refactored to allow the recomputation on demand. Would you like to attempt that refactoring or would you rather me to take a look into it?

@haixuanc haixuanc closed this Mar 5, 2013

haixuanc commented Mar 5, 2013

Sorry about the belated reply. So you mean that we need to refactor the entire code of grid list, or just the row item part? In your comments on pull request #702 , "group.appendRow(row)" you mentioned to apply style to a row before being appended to a group. Can we do the same thing, to re-apply style to a row after setting its number of columns?

@haixuanc haixuanc reopened this Mar 5, 2013

Member

tneil commented Mar 5, 2013

Basically the grid is only setup to do measurements when it parses on the first pass. In order to do dynamic updates like changing the number of columns in a row, the row's items need to be re-computed and layed out.

Updating only the DOM attributes for column numbers won't result in the grid changing its current appearance.

haixuanc commented Mar 5, 2013

So we only need to recompute the style of the row like initializing the row when the grid list is initially created?

haixuanc commented Mar 5, 2013

I mean to repeat the process of styling and laying out the row items when the number of columns is changed. Is my understanding correct?

Member

tneil commented Mar 5, 2013

Correct...

Since many of the grid updates require re-laying out a row or group. My suggestion is to add a recomputeLayout() function on a row and group for the grid. This would be used to set dimensions etc. so that when something is changed it can simply be re-run, instead of duplicating the layout code.

haixuanc commented Mar 5, 2013

Ok, I have already tried out this approach as group.styleRow(row) inside group.appendRow(row) as you suggested. styleRow(row) will recompute and apply the style and layout of the items inside a row. So I think I can use the same approach for the row interface. I will submit my appendRow(row) commit later today. You can take look at it tomorrow to see if it is the right way to go. Then I can try that on this issue as well.

Member

tneil commented Mar 5, 2013

Cool, sounds good

Member

tneil commented Mar 5, 2013

Also make sure you sync up with the latest.. I just checked in some changes for context menus on grids.

haixuanc commented Mar 6, 2013

I created a gist to show the group.styleRow(row) function: https://gist.github.com/haixuanc/5097026
Please take a look at it.

@tneil tneil closed this Jun 26, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment