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

QSelect - fix #5395: issue on IE where q-select does not show menu on click #5396

Merged
merged 1 commit into from Oct 25, 2019

Conversation

@huy-tran
Copy link
Contributor

huy-tran commented Oct 25, 2019

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Documentation
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

  • It's submitted to the dev branch and not the master branch
  • When resolving a specific issue, it's referenced in the PR's title (e.g. fix: #xxx[,#xxx], where "xxx" is the issue number)
  • It's been tested on a Cordova (iOS, Android) app
  • It's been tested on a Electron app
  • Any necessary documentation has been added or updated in the docs (for faster update click on "Suggest an edit on GitHub" at bottom of page) or explained in the PR's description.

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

Other information:

@huy-tran huy-tran changed the title QSelect - fix #5395: IE issue does not show menu on click QSelect - fix #5395: issue on IE where q-select does not show menu on click Oct 25, 2019
@@ -919,6 +919,8 @@ export default Vue.extend({
},
click: e => {
// label from QField will propagate click on the input (except IE)
if (e.target.classList.contains('no-outline')) return

This comment has been minimized.

Copy link
@pdanpdan

pdanpdan Oct 25, 2019

Collaborator

Can you please try with this:

          // label from QField will propagate click on the input (except IE)
          if (
            this.hasDialog !== true &&
            (
              (this.useInput === true && e.target.classList.contains('q-select__input') !== true) ||
              (this.useInput !== true && e.target.classList.contains('no-outline') === true)
            )
          ) {
            return
          }

This comment has been minimized.

Copy link
@huy-tran

huy-tran Oct 25, 2019

Author Contributor

@pdanpdan

Hi mate,

I have tried your changes and it works exactly the same. Just wondering why do we check this.useInput !== true?

on IE e.target.classList.contains('no-outline') only captures the click event when you don't use use-input prop. It would never reach e.target.classList.contains('no-outline') when you use use-input so in my opinion the condition here is unnecessary.

However, since your change works perfectly fine so it's up to your preference. It's great that we can solve this issue anyway.

Cheers!

@rstoenescu rstoenescu merged commit 80b7531 into quasarframework:dev Oct 25, 2019
@rstoenescu

This comment has been minimized.

Copy link
Member

rstoenescu commented Oct 25, 2019

Hi,

Thanks for contributing!
Fix will go into "quasar" v1.2.6. Will make some adjustments though.

rstoenescu added a commit that referenced this pull request Oct 25, 2019
@huy-tran huy-tran deleted the huy-tran:fix/5395 branch Oct 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.