Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix(ui): prevent disabled styling on disabled=false #17527

Merged

Conversation

Evertvdw
Copy link
Contributor

Currently the default Quasar styling will also style elements that have disabled=false as attribute value due to the generic [disabled] attribute selector that is used. This is not correct behavior, this PR fixes that.

What kind of change does this PR introduce?

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

Does this PR introduce a breaking change?

  • Yes
  • No

The PR fulfills these requirements:

  • It's submitted to the dev branch (or v[X] 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 an Electron app
  • Any necessary documentation has been added or updated in the docs 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 start a new feature discussion first and wait for approval before working on it)

Other information:

@rstoenescu rstoenescu merged commit 67a9b71 into quasarframework:dev Oct 19, 2024
@rstoenescu
Copy link
Member

Thanks!

@rstoenescu
Copy link
Member

This will go into Quasar 2.17.1

@rstoenescu
Copy link
Member

Reverting because:

  1. The documentation for "disabled" attribute does not say it is a boolean. It just mentions that it has effect when it is present. https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/disabled
  2. The selector in visibility.sass becomes too specific and breaks things all over the UI.

@Evertvdw
Copy link
Contributor Author

Evertvdw commented Nov 6, 2024

Reverting because:

  1. The documentation for "disabled" attribute does not say it is a boolean. It just mentions that it has effect when it is present. https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/disabled
  2. The selector in visibility.sass becomes too specific and breaks things all over the UI.

Reading the Html spec it seems that the disabled=false pattern is just bad Html. I will contact the external tool which has this issue to get them to just omit the disabled attribute when it should not be disabled.

Thanks!

@rstoenescu
Copy link
Member

Exactly. Yes

@ReneBrandt
Copy link

ReneBrandt commented Dec 24, 2024

I was on the verge of creating a new issue for this (let me know if I should!), but I think the current state (not being able to bind a boolean directly to disabled) is breaking with Vue's Boolean Casting and how this normally works in vanilla Vue. Note Vue specifically uses this prop disabled as an example in the docs, stating :disabled="false" is equivalent to not providing it at all. That is no longer the case now when working in Quasar.

I also think the styling oversteps its boundaries, applying the disabled visualization also to plain

components. See https://jsfiddle.net/35jz9am7/31/, which makes it even more tricky.

I will move away from using disabled for now as much as possible, but 1) it should be considered a breaking change, and 2) I would request this to be reconsidered in the future, due to the breakage with very typical Vue convention (despite this not being in-line with the HTML spec) and thus possibly breaking with lots of libraries applying this typical Vue use-case

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants