Conversation
|
Ready for review |
|
Are there tests for the |
| } | ||
| } else if (tableFillupMode === TABLE_FILLUP_MODE_LAST_COLUMN) { | ||
| // Add all delta to last column | ||
| columns[columns.length - 1].set('width', columns[columns.length - 1].get('width') + delta); |
There was a problem hiding this comment.
I would store columns[columns.length - 1] in a variable to make this line easier to read (you're doing it right above)
| }} | ||
| {{#ember-table-row | ||
| row=t | ||
|
|
|
Tests added. Comments addressed |
| }); | ||
|
|
||
| test(`Test ${headerTest}column fill up - none mode`, async function(assert) { | ||
| await setupFullTable(this, {}, { headerComponent: customHeader }); |
There was a problem hiding this comment.
I would enforce the mode here instead of making assumptions about the default value of the template.
|
|
||
| assert.ok( | ||
| Math.abs(tableWidth - firstColumnWidth - 180 * (headerCount - 1)) <= 1, | ||
| 'First column takes extra sapce in first column fill up mode.'); |
|
|
||
| const headerCount = findAll('.et-thead tr th').length; | ||
| for (let i = 1; i <= headerCount; i++) { | ||
| assert.equal(tableHelpers.getHeaderElement(i).offsetWidth, 180, |
There was a problem hiding this comment.
I would create a constant in the test file otherwise it's not easy to understand where this value comes from
There was a problem hiding this comment.
Also the sample data doesn't seem to allow to fully test that option, no? (why would they all be the same unless the test data forces that behavior?)
| const headerCount = findAll('.et-thead tr th').length; | ||
| for (let i = 1; i <= headerCount; i++) { | ||
| assert.equal(tableHelpers.getHeaderElement(i).offsetWidth, 180, | ||
| 'Table header have same width in proportional fill up mode.'); |
|
Comments addressed |
| } | ||
| } else if (tableFillupMode === TABLE_FILLUP_MODE_EQUAL_COLUMN) { | ||
| // Split delta by their proportion. | ||
| const columnDelta = delta / columns.length; |
There was a problem hiding this comment.
do we need to do something for the remainder?
There was a problem hiding this comment.
This is real number division. You do not have to worry about remainder.
|
Bump |
| * 3) "first_column": extra space is added into the first column. | ||
| * 4) "last_column": extra space is added into the last column. | ||
| */ | ||
| @property tableFillupMode = TABLE_FILLUP_MODE_NONE; |
There was a problem hiding this comment.
Let's call this tableResizeMode, without context I don't think people will understand what "fillup" means here, whereas "resize" gives them a bit more of a hint
|
Name changed. |
pzuraq
left a comment
pzuraq
left a comment
There was a problem hiding this comment.
Awesome, looks good to merge!
billy-addepar
force-pushed
the
billy/fillup-columns
branch
from
25702c0 to
a976773
Compare
3 participants
Add table fill up mode for table when total columns width does not match table width. The extra space can go to either the first column or be distributed equally among all columns.