vue-material
vue-material copied to clipboard
Fix/md tabs/ordering
This merge request fixes #2282 (and possibly also fixes #2015).
I heavily rebased to have one (small, meaningful and easy-to-review) commit per fix/feat. Tell me what you think about it all.
There is a remaining WIP commit. That commit is for you to visually test the behaviors before (by checking out the commit) and after (by checking out the branch) the bug fixes (same goes for unit tests before and after). This big example lists all the possible edge case I could find, in order to do my best to not introduce a regression in the library. I will remove that commit before merging. Or if you have a private testbed where I could put the example's content, it would be even cooler: it is so easier to visually test weird edge-cases are correctly handled... Unit tests are good for that too, but it's always nice to see it visually (and I didn't succeed to unit-test indicatorStyles generation)
About the implementation of ordered tabs (and numeric IDs): basically, there is no way for MdTabs or MdTab to know the order of tabs in the MdTabs' slot. The only way I found is querying the DOM. So I made MdTabs adds an ID object to the DOM object (and not an attribute, so the Number/String/Other nature of the ID is kept, and not transformed to a String when converted to a DOM-node attribute). From this, I can compute an orderedIds array and base all order-sensitive computations on it (rendering, activation by index, swipe...).
There was a few bugs I found and fixed:
- activeTab is an ID, but it sometimes contained an index, and a double-bug reverted the bug sometimes, but not always
- missing null check on a duplicate activeTabIndex set
- the selector
.md-tab:nth-child(${this.activeTabIndex + 1})
did match sub-md-tabs too, and depending on the order of elements, a sub-tab's height could be misued as the height og the current tab - md-changed is emitted twice when md-active-tab changes
Hi @slaout thank you for your PR. Sorry for the late response. I like your solution. Can you come back with an update on docs content? On the documentation page, you should put the "Tabs ordering" section and update the props table.
After your update, we come back with a new review and approve the PR.
All the best, Ben
Hi, thanks for your reply: no problem.
I'll look at your suggestions. But to be sure we're on the same page:
By 'put the "Tabs ordering" section', you mean I remove this testing section, not put it in a more permanent place?
And by 'update the props table' I tought I changed the type to "Number | String": what do you mean? (I have not the code under my eyes right now: it may or may not be obvious when I will read it).
Done:
- I've rebased without the WIP commit showcasing the bugs before and after the fixes
- I've added a commit to document the new behavious
- As I was not sure about what you meant, I've added yet another commit to show a more user-friendly example of how tabs ordering work with dynamic tabs (can be squashed with the previous commit if it's OK with you, or removed if you did not mean to document this behaviour).
Sorry for the delay.
Looking forward for the new review :-)