Fix column alignment issue when new rows were added after first render #36
Conversation
|
Changes Unknown when pulling bc5dca9 on AdamWard1995:flexTable into ** on ciena-frost:master**. |
AdamWard1995
changed the title
|
Changes Unknown when pulling 3756eb0 on AdamWard1995:flexTable into ** on ciena-frost:master**. |
addon/mixins/table.js
Outdated
| * @returns {String} jQuery selector string for getting the header column row | ||
| */ | ||
| headerColumnsSelector (haveCategories) { | ||
| return `${haveCategories ? '.frost-table-header-columns' : ''}` |
There was a problem hiding this comment.
Why isn't this code just return haveCategories ? '.frost-table-header-columns' : ''?
addon/components/frost-table-row.js
Outdated
| this.$().css({ | ||
| 'flex-grow': 1, | ||
| 'flex-shrink': 0, | ||
| 'flex-basis': `${this.setMinimumCellWidths('')}px` |
There was a problem hiding this comment.
Does the '' need to be passed? Can't is just be${this.setMinimumCellWidths()}px?
whereas the |
|
@notmessenger I had originally wrote it as using didInsertElement so that the cells would only be aligned once. As I tried to integrate this in mcp-ui I noticed that as rows are added and removed through filtering for example, the new rows didn't get aligned |
|
@AdamWard1995 are all (re)calculations needed or just some of them? If all that's fine, just seems while glancing at it (only) that minimum widths would not need to be, for example. |
|
So in the case that a new row is added, a cell may be wider than the current minimum width in which case the whole column needs to be realigned. I can play for a bit and see if it is possible to only do the alignment when this situation occurs |
| @@ -84,6 +84,7 @@ | |||
| }} | |||
| {{#each items as |item index|}} | |||
| {{frost-table-row | |||
| class=(if isSelectable 'selectable' '') | |||
There was a problem hiding this comment.
Shouldn't this be done via the classNameBinding https://github.com/ciena-frost/ember-frost-table/blob/master/addon/components/frost-table-row.js#L20 ?
There was a problem hiding this comment.
so it is in frost-table-row, but in this case don't want to set isSelectable to true for middle and right sections otherwise they'll get their own selection cell too. Only want a selection cell in the left section
There was a problem hiding this comment.
....but we want the middle and right section rows to have this class so that the cursor pointer changes
|
@notmessenger did a best effort to minimize the number of DOM accesses. It should only realign the entire table when the number of rows changes |
|
Changes Unknown when pulling e59d04c on AdamWard1995:flexTable into ** on ciena-frost:master**. |
This project uses semver, please check the scope of this pr:
CHANGELOG