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(vlist.sass): fix menu list items on IE11 #10717

Merged
merged 5 commits into from
Mar 10, 2020
Merged

fix(vlist.sass): fix menu list items on IE11 #10717

merged 5 commits into from
Mar 10, 2020

Conversation

Mikilll94
Copy link
Contributor

On IE11 text in menu list items was not vertically centered.

fix #10464

Description

Fixes #10464

On IE11 text in menu list items is not vertically centered. This PR fixes this issue.

Motivation and Context

I wanted to fix the bug which was occurring only on IE11.

How Has This Been Tested?

Because this is a visual bug I have not created any unit tests.

I have tested this PR manually. First I have checked if on IE11 the behavior is correct. Then I have checked on Chrome and Firefox to see if everything is working as before.

Markup:

<template>
  <v-container>
    <div class="text-center">
        <v-menu offset-y>
          <template v-slot:activator="{ on }">
            <v-btn
              color="primary"
              dark
              v-on="on"
            >
              Dropdown
            </v-btn>
          </template>
          <v-list>
            <v-list-item
              v-for="(item, index) in items"
              :key="index"
              @click="onItemClicked"
            >
              <v-list-item-title>{{ item.title }}</v-list-item-title>
            </v-list-item>
          </v-list>
        </v-menu>
      </div>
  </v-container>
</template>

<script>
export default {
  data() {
    return {
      items: [
        { title: "Click Me" },
        { title: "Click Me" },
        { title: "Click Me" },
        { title: "Click Me 2" }
      ]
    }
  },
  methods: {
    onItemClicked() {
    }
  }
}
</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)

On IE11 text in menu list items was not vertically centered.

fix #10464
@DRoet DRoet added C: VList VList platform specific The issue only occurs on a specific platform labels Mar 2, 2020
@DRoet DRoet requested a review from johnleider March 2, 2020 12:30
@johnleider
Copy link
Member

johnleider commented Mar 3, 2020

Was this a brute force fix or is there some flex knowledge you can bestow upon me?

Fixed vertical text alignments in lists on IE11

fix #10464
@Mikilll94
Copy link
Contributor Author

@johnleider
Yeah, that was a brute force fix 😄 I removed it and added next commit with a real solution.

I have finally found the real cause of this bug. It is because on IE11 align-items: center is not working when min-height is set. This is described in details here:
philipwalton/flexbugs#231

I have applied this workaround:
philipwalton/flexbugs#231 (comment)

Please check my changes again.

Just applied refactoring from comment.

fix #10464
DRoet
DRoet previously approved these changes Mar 4, 2020
Copy link
Member

@johnleider johnleider left a comment

Choose a reason for hiding this comment

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

Psuedo elements should use 2 colons

packages/vuetify/src/components/VList/VListItem.sass Outdated Show resolved Hide resolved
Pseudoelements should you double colon + removed unnecessary new line

fix #10464
@johnleider johnleider merged commit 81dfbf8 into vuetifyjs:master Mar 10, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Apr 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
C: VList VList platform specific The issue only occurs on a specific platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug Report] IE11 - menu list items are not vertically centered
3 participants