vuematerial/vue-material

[BUG] MdTabs: Tabs with number-like id are wrongly ordered by id

slaout opened this issue · 6 comments

Steps to reproduce

When md-tabs are Numbers or Strings having a Number value, tabs are ordered by id instead of by appearance order in the source code.

Which browser?

Chrome on Windows.

What is expected?

We declare tabs in a certain order in the Vue template: we expect to display the tabs in the same order.

What is actually happening?

Tabs are sorted by Number ids before displaying them.

Reproduction Link

https://codesandbox.io/embed/vue-material-quick-start-forked-buewe

Here is the relevant part:

<template>
  <md-tabs>
    <md-tab :id="1" md-label="A">A</md-tab>
    <md-tab :id="2" md-label="B">B</md-tab>
    <md-tab :id="0" md-label="C">C</md-tab>
  </md-tabs>
</template>

Tabs do not appear in the order "A B C", but in the order "C A B".

Possible Solution

I think this is due to the implementation of storing tabs in an Object and using its keys:

https://github.com/vuematerial/vue-material/blob/dev/src/components/MdTabs/MdTabs.vue#L81

      MdTabs: {
        items: {}
      },

I think you could resolve this by using a new Map() instead of a raw {}.

I base my assumption on this test:

a = {}; a["c"] = ""; a["a"] = ""; a["b"] = ""; Object.keys(a);
// Outputs ["c", "a", "b"]

a = {}; a[1] = ""; a[2] = ""; a[0] = ""; Object.keys(a);
// Outputs ["0", "1", "2"]

a = {}; a["1"] = ""; a["2"] = ""; a["0"] = ""; Object.keys(a);
// Outputs ["0", "1", "2"]

a = new Map(); a.set(1, ""); a.set(2, ""); a.set(0, ""); a.keys();
// Outputs MapIterator {1, 2, 0}

Here is an additional side-effect of using an Object and Object.keys:

<md-tabs>
  <md-tab id="a1" md-label="A">A</md-tab>
  <md-tab id="a2" md-label="B">B</md-tab>
  <md-tab id="a3" md-label="C" v-if="additionalColumn">C</md-tab>
  <md-tab id="a0" md-label="D">D</md-tab>
</md-tabs>

On component mount, additionalColumn is false, and displayed tabs are "A B D"... When additionalColumn becomes true, displayed tabs are in the wrong order: "A B D C"

You can play with this code here:
https://codesandbox.io/s/vue-material-quick-start-forked-7z11t

Hello,

I've noticed the "help wanted" tag.
So you are OK to make the tab-ordering breaking-change as described?
If I look at it and open a pull request, it could be accepted and released "quite soon"?

Best regards, and thanks for the great work you've already done,
Sébastien Laoût.

Hi, yes if you come with PR and it is ok, we will accept it.

Hello, just to let you know I've finally got time to start working on it... I have a somewhat working solution for numeric IDs, but I would like to also solve the problem of tabs being toggled on and off in the middle of the bar, and this would change the solution a lot. This is still a work in progress: I'll keep you posted as I advance...

Happy new year: here is my (late) Christmas present: the pull request:
#2298

Hello @marqbeniamin

I see you are very busy. I hope it's because of good personal (or professional) projects.

Is there anything I can do for you to ease the PR validation?