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(VTextField): legend position not aligned with reverse prop #13051

Merged
merged 2 commits into from
Feb 5, 2021

Conversation

dchinu97
Copy link
Contributor

@dchinu97 dchinu97 commented Feb 4, 2021

Description

Legend position was not aligned with the reverse prop that reverses the label position. So when the value of the reverse prop is true the legend should move right
fixes #10798

Motivation and Context

Done to Fix #10798

How Has This Been Tested?

Visually via playground on Chrome, Edge, IE and Morzilla

Markup:

<template>
  <v-container>
    <v-text-field label="Label" outlined reverse></v-text-field>
    <v-text-field label="Label" outlined></v-text-field>
  </v-container>
</template>

<script>
  export default {
    data: () => {
      return {}
    },
  }
</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 features but makes things better)

Checklist:

  • The PR title is no longer than 64 characters.
  • The PR is submitted to the correct branch (master for bug fixes and documentation updates, dev for new features and backwards compatible changes and next for non-backwards compatible changes).
  • My code follows the code style of this project.
  • I've added relevant changes to the documentation (applies to new features and breaking changes in core library)

@KaelWD
Copy link
Member

KaelWD commented Feb 4, 2021

Turns out this was already fixed in #10278, the issue you commented on was missing <v-app>. Thanks anyway.

Sorry I missed that this was specific to firefox. My other comment still stands however.

@KaelWD KaelWD closed this Feb 4, 2021
@@ -371,6 +371,9 @@ export default baseMixins.extend<options>().extend({
style: {
width: !this.isSingle ? convertToUnit(width) : undefined,
},
attrs: {
align: this.reverse ? 'right' : 'left',
Copy link
Member

@KaelWD KaelWD Feb 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would've asked you to redo with CSS anyway, this attribute is deprecated and doesn't account for RTL.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Firefox bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1488228

Looks like using margin instead of text-align works in firefox.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @KaelWD , Will verify the same and update the PR

Copy link
Contributor Author

@dchinu97 dchinu97 Feb 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@KaelWD As mentioned in the Mozilla patch here https://bugzilla.mozilla.org/attachment.cgi?id=9006391

image

These changes seems to fix it.

@@ -433,10 +433,12 @@

&.v-text-field--reverse legend
+ltr()
text-align: right
margin-inline-start: auto
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IE doesn't support this property

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @jacekkarczmarczyk. Didn't knew about that, Update and tested on Chrome, Edge, IE and Firefox

fix(VTextField): legend position not aligned with reverse prop
@KaelWD KaelWD merged commit 9e93f7d into vuetifyjs:master Feb 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants