Don't offset last tab when scrolling

[smiller171]

fixes #3294

Description

Check if active tab is last in list and don't add an offset when scrolling if it is.

Motivation and Context

When using grow sometimes tabs would get scrolled (and offset) incorrectly.
This change also improves look and feel of normal scrolling tabs

How Has This Been Tested?

visually

Markup:

<template>
  <v-app>
    <v-toolbar
      dark
      dense
    >
      <v-tabs
        grow
      >
        <v-tabs-slider></v-tabs-slider>
        <v-tab
          v-for="item in tabs"
          :key="item.key"
        >
          {{ item.label }}
        </v-tab>
        <v-tab
          v-for="(item, index) in tabs"
          :key="index + 3"
        >
          {{ item.label }}
        </v-tab>
        <v-tab
          v-for="(item, index) in tabs"
          :key="index + 6"
        >
          {{ item.label }}
        </v-tab>
      </v-tabs>
    </v-toolbar>
  </v-app>
</template>

<script>
export default {
  data() {
    return {
      tabs: [
        {
          key: 'about',
          label: 'About',
          target: '/about/'
        },
        {
          key: 'resume',
          label: 'Resume',
          target: '/resume/'
        },
        {
          key: 'contact',
          label: 'Contact',
          target: '/contact/'
        }
      ]
    }
  }
}
</script>

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Improvement/refactoring (non-breaking change that doesn't add any feature but make things better)

Checklist:

• The PR title is no longer than 64 characters.
• The PR is submitted to the correct branch (master for bug fixes, dev for new features and breaking changes).
• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have created a PR in the documentation with the necessary changes.

[codecov]

Codecov Report

Merging #3777 into master will decrease coverage by 4.88%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #3777      +/-   ##
==========================================
- Coverage    89.6%   84.71%   -4.89%     
==========================================
  Files         206      152      -54     
  Lines        4867     3527    -1340     
  Branches     1223     1105     -118     
==========================================
- Hits         4361     2988    -1373     
+ Misses        408      406       -2     
- Partials       98      133      +35

Impacted Files	Coverage Δ
src/components/VTabs/VTabs.js	100% <100%> (ø)	⬆️
src/components/VRadioGroup/VRadioGroup.js	41.17% <0%> (-58.83%)	⬇️
src/components/VDataIterator/index.js	50% <0%> (-50%)	⬇️
src/mixins/detachable.js	47.36% <0%> (-45.74%)	⬇️
src/components/VCheckbox/VCheckbox.js	65% <0%> (-35%)	⬇️
src/components/VRadioGroup/VRadio.js	66.66% <0%> (-33.34%)	⬇️
src/mixins/selectable.js	64% <0%> (-29.88%)	⬇️
src/components/VSlider/VSlider.js	72.41% <0%> (-26.91%)	⬇️
src/components/VDataTable/VEditDialog.js	0% <0%> (-20%)	⬇️
src/components/Vuetify/util/goTo.js	78.12% <0%> (-16.17%)	⬇️
... and 231 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f32242a...97419e4. Read the comment docs.

[smiller171]

I don't agree with codecov here. Ideally there should be a special test case for the last tab.

[johnleider]

This doesn't actually resolve the underlying issue. The problem is scrollOffset is not being properly reset when isOverflowed changes, which happens when going from mobile to desktop.

[johnleider]

My current proposition for this fix: #3781. After running through, this ends up being more of a feature request. If I'm wrong please correct me.

[smiller171]

@johnleider I think you're correct, though I believe this solves the presentation of the underlying issue as a side effect. That said, I would think that scrolling past the end in any situation is still a bug, as it sends a signal to the user that there are more options off to the right when there are not, and it's inconsistent with the behavior of the overscroll on the left side since the left side will not overscroll beyond the first tab.

[johnleider]

It's only a bug if it wasn't intentional by us. The offset is intentional. Your logic in regards to it being a worse user experience, that may be, but this should still be a feature request to dev branch.

[smiller171]

Sure. I wasn't sure scrolling on the last tab was intentional. I've updated the PR to target the dev branch.

[KaelWD]

Does this still apply, and if so can you explain what it's actually about? The original issue appears to be fixed now.

[smiller171]

@KaelWD I'll have to build a new test case. Something has changed that radically altered how my codepen renders.

[smiller171]

@KaelWD No, this isn't fixed. I'll get a codepen together in a minute, but the short version is that if you have 6 tabs and only room on screen for 4 tabs, when tab 5 is selected you expect the bar to scroll so that you can see part of tab6, so you know something is there, but you would expect that when tab 6 is selected the right edge of tab 6 should line up with the right edge of the tab bar so that it's clear you're at the end of the list of tabs. Instead the behavior seen is that there is a gap between the right edge of tab 6 and the right edge of the bar, making it appear to the user as if there is another interactive tab when there is not.

[smiller171]

@KaelWD Here's an example pen. Make the view narrow enough to overflow the tabs and select tab 10 to see what I mean

https://codepen.io/smiller171/pen/MXrjEq

[KaelWD]

That's what the spec says should happen, it's actually incorrect because the left side is supposed to have that offset too.

[smiller171]

Could you point to the spec? This behavior increases user friction and as far as I can tell it deviates from every other material design implementation, including those by Google.

[KaelWD]

I guess it doesn't say anything about the right-hand side, just that the left should align with the keyline. I don't really mind either way so I'm ok to merge this if the bossman is alright with it.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. Please direct any non-bug questions to our Discord