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

fix(VDialog): Do not override typography defined for VCard #19688

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

J-Sek
Copy link

@J-Sek J-Sek commented Apr 24, 2024

Description

VDialog overrides line-height and font-size forcing me to add very specific CSS overrides to make the UI consistent. inherit in context of CSS does not mean "take previous value", but rather "pull value from parent element".

In the example below, the effective line-height is 1.5 defined on the html level for the base font size 16px it then resolves to 24px.

v-card-text-2024-04-24_19-08

The same VCard outside of VDialog has smaller font size (14px) and line height 1.25rem (20px).

Variables $card-text-font-size and equivalent for line-height date back to October 2019. Dialog got these inheirt before 3.0.0-alpha in June 2021 as part of a wider fix of styling inconsistencies and $dialog-card-title-font-size was removed, while equivalent for line-height supposedly never existed.

I get why cards inside dialog should have larger text, but since I find normal cards unnecessarily small and already bump it up with dedicated variable, it would be nice to avoid this tricky override. The code below could be simplified, but reduction could bite me later and I don't like this feeling.

.v-dialog > .v-overlay__content > .v-card > .v-card-text,
.v-dialog > .v-overlay__content > form > .v-card > .v-card-text
  font-size: $card-text-font-size
  @include card-line-height-densities($card-text-density-line-height)

Another consideration is Material Design specification. I am not familiar with your sources for specification. A quick search reaveals that M2 is consistent with current approach [1] (font size 16px, line height 24px), but M3 looks like the text is smaller [2] and in line with Card.

A brief search did not reveal any issues that might cover this problem. I have skipped to PR, but I would be glad to discuss and adapt the changes.

@MajesticPotatoe MajesticPotatoe added T: bug Functionality that does not work as intended/expected C: VDialog VDialog labels Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: VDialog VDialog T: bug Functionality that does not work as intended/expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants