Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug Report][3.6.1] VDataTable mobile mode is based on screen width, not container width #19726

Closed
snazzyfox opened this issue Apr 30, 2024 · 4 comments · Fixed by #19744
Closed

Comments

@snazzyfox
Copy link

Environment

Vuetify Version: 3.6.1
Last working version: 3.5.18
Vue Version: 3.4.26
Browsers: Firefox 125.0
OS: Windows 10

Steps to reproduce

I've included two tables in the playground. Resize the playground preview window to see the effects. When the preview window is under 1280px wide (the now-default mobile breakpoint), both tables go into mobile mode.

Expected Behavior

The table goes into mobile mode when the table itself is below a certain specified width.

Mobile mode should be disabled by default to ensure backwards compatibility.

Actual Behavior

The table goes into mobile mode based on the viewport size, regardless of its own size.

Mobile mode is enabled by default under 1280px viewport size

Reproduction Link

https://play.vuetifyjs.com/#...

Other comments

I don't think it should be an assumption that VDataTable always matches page width - most sites do not have it covering the whole screen edge-to-edge.

This change is causing some of my apps to now display mobile table on desktop with no code change, and since it only applies to certain screen sizes, it's very easy to miss by the dev. The width of 1280px may also be too wide since it's easy to go under that on desktop browsers or especially in something like an electron app.

@snazzyfox
Copy link
Author

Adding a couple screenshots to illustrate my point: with the vuetify docs site itself, at 1280x720, the example table is perfectly readable:

image

When I reduce the viewport by 1px, the table goes into mobile mode, which now makes the page completely unreadable as the rest of the page is still designed for desktop mode:

image

@webdevnerdstuff
Copy link
Contributor

webdevnerdstuff commented May 2, 2024

You can set the mobile prop to false and it will disable it. You can also specifically set which breakpoint it should break to mobile by setting the same prop using useDisplay. 1280 is a default mobile size so that is correct, so personally I like to set it as smAndDown.

Basic example

@snazzyfox
Copy link
Author

Thanks. Yes, I understand that this can be adjusted or disabled manually so a workaround exists.

I'd still consider this a bug though since it's not working "as advertised". I think it makes the most sense for the mobile layout to be disabled by default and only enabled if a breakpoint is explicitly set. Other components such as stepper is already done this way. There are a few reasons for it:

  • DataTable isn't a page layout component and don't have to scale with the screen, so it doesn't make sense to change layouts based on the viewport size. The desktop and mobile layouts are also significantly different which means they may require separate design attention. This is what sets it apart from most other automatically-responsive components like nav drawer, toolbar, etc.
  • The breakpoint isn't one-size-fits-most unlike other components. What fits in a certain width varies depending on the content inside. So picking 1280px arbitrarily is counter-productive since this default isn't necessarily "better most of the time".
  • It's more likely for someone to leave the default in by accident, than actually wanting the default behavior. Someone designing for mobile would be already testing at different sizes and setting breakpoints anyways. But someone who doesn't care about mobile likely won't even know the mobile layout is a feature, and end up leaving it in resulting in a bug down the line.
  • It's also a silent behavior change from 3.5, so it can easily be added to existing apps by accident.

@webdevnerdstuff
Copy link
Contributor

You do make some good points to be considered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants