vuetifyjs / vuetify

🐉 Vue Component Framework
https://vuetifyjs.com
MIT License
39.89k stars 6.97k forks source link

[Bug Report][3.3.2] VSkeletonLoader missing type `table-cell` #17506

Open aentwist opened 1 year ago

aentwist commented 1 year ago

Environment

Vuetify Version: 3.3.2 Vue Version: 3.2.47 Browsers: Firefox 113.0 OS: Ubuntu undefined

Steps to reproduce

<VSkeletonLoader type="table-cell" />

Expected Behavior

bone is defined

Actual Behavior

bone is undefined

🦴🦴🦴🦴🦴

infinite loop with warnings crashes the tab

Other comments

type="text" works fine, for instance

MajesticPotatoe commented 1 year ago

there is no type table-cell https://vuetifyjs.com/en/components/skeleton-loaders/#type

aentwist commented 1 year ago

Hm. True. Where did it go? Should I do other things to emulate it's behavior (I need text, but inline)?

In the docs

table-row -> table-cell@6

https://vuetifyjs.com/en/components/skeleton-loaders/#type

  1. What does table-row actually expand to now?
  2. What is the recommended way to accomplish what table-cell used to do?

@MajesticPotatoe

MajesticPotatoe commented 1 year ago

table-row presents itself the same, table-cell just doesn't exist, and looks to just be replaced with text table-row -> table-cell@6 => table-row -> text@6

https://github.com/vuetifyjs/vuetify/blob/master/packages/vuetify/src/labs/VSkeletonLoader/VSkeletonLoader.tsx#LL51C25-L51C25

eg: https://play.vuetifyjs.com/#eNqlkT9PwzAQxb+K8RKQmlgCpiqtysbAhgRD3SFNrmDqf7KvgarKd8d2ghSlElRiyz3fvd+9y/pEH6wt2gPQOS0RlJUVwpJrQso2r6xNn6mojcZKaHCDFMSX5z1IQKOfTNWAI3i0sOAUq62E3JlPTpclmzT9PQ1f+MtgyaarRKXftGSjBKH0tRMWiQc89EGEssYhOREHO9KRnTOKZCF8FtsJCb4eifJvZBE7rrNHkNKQV+Nkc5XdREBvGczojPZuuaps8eGNDhc8RRc+PHhO5yQpUQuUWHP6jmj9nLG60WGsASlaV2hApq1iq9DG3EGjUJA3Rq3uirvinjXC41guwKt8Gy7swQUTTmcjDIrd8QLU0JkItz1hkHJZbX1EnFmzwGvB5Q50+CPgLk00GRunmjydJYv0jusu3FsKvfeTU9c+nXn9s+S/Iiez6LSJwG5G056BnAbo5hvcxhNv

aentwist commented 1 year ago

@MajesticPotatoe table-row != text@6 🤔 therefore I am left with nothing to customize the number of columns if I want the skeleton to be somewhat precise (i.e. text@7). Should I open a new issue for this (unexpected?) behavior?

text@6 - what is with those widths?!

Screenshot from 2023-05-31 16-17-39

table-row

Screenshot from 2023-05-31 16-18-04

Edit: wow! Thanks for showing me Vuetify Play. Here is the reproduction.

MajesticPotatoe commented 1 year ago

it does = text@6, it just has some extra styling in the context of the table-row alias. I'll leave it up to @johnleider how he wants to proceed with this, whether to bring back table-cell or create a inline-text alias

johnleider commented 1 year ago

We can add it back, nbd.

aentwist commented 1 year ago

I'm not sure how useful it is, but I feel like including some inline text type, such as table-cell, might provide completeness with the current suggested usage.

Potential workaround: d-inline many VSkeletonLoader with trivial types. Are there disadvantages to using many VSkeletonLoader vs. one with many types in Vue 3? Unlikely. This calls into question the design of the VSkeletonLoader type prop overall - which at least made sense for Vue 2 (if not Vue 3?). Simplifying this would be breaking and so is not possible at this time.

Another related counterpoint about tables. Using type table-cell with the loading slot feels good, but the best way for the VSkeletonLoaders to have accurate sizing is to use them inside td cells. So really tables might be better served by not using the loading slot and using v-if inside each cell to display a single loader. Again suggests many components each having base types. Potential feature request.

Thoughts?

Mentioning this because I was looking for the exposed table-cell type for prototyping but would actually prefer the inside-td method described above for production if the widths don't line up well. Not trying to mislead you with this use case, although it is completely valid. And if there is a clear downside to using many VSkeletonLoader instead of one with many types - the tradeoff of correct "cell" widths vs. one component with e.g. type table-cell@7 is a choice users should be able to make.

MajesticPotatoe commented 1 year ago

could always just bring back the types prop (which let you defined your own custom types from the base presets, that are merged into type).

I think creating a inline-text type would probably be better though, and then table-row could just be inline-text@6, removing the table specific styling and making it more versatile. Then if you wanted to really create something custom, you could just define your own / override existing default type using the types prop