quasar
quasar copied to clipboard
quasarframework
q-btn in q-field is incorrectly triggering click event
What happened?
q-btn in q-field is incorrectly triggering click event. Clicking on an area outside q-btn will also trigger the click event of q-btn. Version 2.13.0 works fine, this problem occurs with version 2.13.1 and above.
What did you expect to happen?
The click event should only be fired when q-btn is clicked.
Reproduction URL
https://codepen.io/gefan/pen/PogGxvG
How to reproduce?
- Go to reproduction link.
- Open the console at the bottom of the page.
- Click on the white area of the q-field, you can see that the click event of q-btn is triggered.
The link below is the same example in version 2.13.0, clicking on the white area will not trigger the click event of q-btn. https://codepen.io/gefan/pen/PogGxMG
Flavour
Quasar CLI with Vite (@quasar/cli | @quasar/app-vite)
Areas
Components (quasar)
Platforms/Browsers
Chrome
Quasar info output
No response
Relevant log output
No response
Additional context
No response
@magnww this happens due to event bubbling if you use @click.stop it will prevent the issue that you are facing. I believe this helps!
@magnww this happens due to event bubbling if you use @click.stop it will prevent the issue that you are facing. I believe this helps!
Thanks for the reply, but nothing changed after adding @click.stop.
@magnww as you mentioned, version 2.13.1 and above has this issue but it can be solved, I've updated your example
Most likely what caused this is getting rid of the for attr for the label tag which is a commit due to fix this issue.
after diving into the codebase I've found out that targetUid is null, meaning for attr is not set when there is no for property that is passed as a prop despite the default tag which is a label. which might have been added intentionally! because of mentioned issue.
we had a browser a11y and autofilling violation which is fixed by now in (v2.13.1 +) but we are facing an event capturing issue which is caused by the solution of the first issue (not setting for attr!).
chrome or browser yellings about a11y violations with an example:
First:
after inspecting the generated HTML we see a label like this in V2.13.0 and below:
but chrome yells at us:
Second:
after inspecting the generated HTML we see a label like this in V2.13.1 and above:
but again chrome warns us:
As you can see, Chromes warning is quiet logical !! when the value of for attr (targetUid) was generated no form control field was binded to it and also when for attr is not setted chrome says label is not associated with a form field !
I was thinking of a label tag with a for attr to be added dynamically based on the provided tag property, but that is also Incorrect, once more we see this in the log:
But this is a solution for this current issue (event capturing) as one might say or at least me but of course not the right approach since it incurs our previous issue! in another word,
for is added with a value of targetUid but no element is bound to it therefore it does not trigger! and if we bind it to a form control field event capturing occurs. (generated for or id is passed via control slot).
Why Event capturing with label tag only ?
div and label tags both work as containers but when it comes to event propagation, they behave differently. not to be misunderstood both propagate according to the standard DOM event flow (bubbling and capturing).
but clicking on the label will focus or activate the associated form control. this behavior can affect how events are perceived to propagate, as clicking the label effectively triggers an event on another element !!
Why we did not have the event capturing issue before ?
we had a11y browser violation but we did not have event capturing since the label was only triggering the event based on the passed for attr which was a unique id f_uid and it did not exist in dom so to fire the event !
that means when we use label tag we should declare a specific for attr like for="username" then the button or input inside the control slot which must be a direct child to it, it should have an id specified with the same value in this case id="username" then it'll work like a clock !
in another word, there are two possible ways:
- QField should act as form control wrapper (somehow with providing both tag as
labelandforproperty binding it to the form field). which incurrs event capturing if we bind it to the form control field. which should be solved ! - QField act as a custom component wrapper (without form control fields i.e. input or button) more like a dummy input wrapper.
Since the QField component is created to work in both cases seamlessly, Its kinda a bit tricky to decide for a concise solution! So I'd like to suggest, giving a warning note in the documentation, describing the edge case briely is gonna be pretty handy !
I hope, it helps to clarify the core concept, Thanks <3
Based on my understanding of this and the codebase, I've come up with an idea. so far, i think it is the right approach and I've created this Pull Request. which closes [ #17005, #16589, #16671, #16883 ]
Remembering the cause of the issue, which is getting rid of for attr, requiredForAttr: false while the tag is a label (up above i mentioned why). so i got to solve it like this:
requiredForAttr: props.tag === 'label'. meaning when the tag is label generate a unique for attr value. luckily it solves this issue, mission accomplished, but wait up !! that violates a11y, right ?
well, yes. that's why i did not stop there !
to prevent this violation, we have to know what QField is for ! which is a form wrapper right ? but a custom component form wrapper or a form control field wrapper ?
let's catch the first and say, a custom component form wrapper, then it is pretty easy we'd pass the tag property as div and now vola the a11y violation vanishes like socks in the dryer!
and for the second we change the tag property to label, which i don't because it is the default value. (before merging this PR).
well not quiet yet, you know there are many softwares using Quasar, without providing the property and they encounter this a11y violation (without even noticing).
even quasar QField examples/usages, we should go and change every one of them and provide the tag property as div when needed (majority cases). so that chrome stops screaming about autofilling !
thus, i wanted the least change, i got another magic touch. and changed the default value of tag property from label to div. with that concept in my mind QField was exposed to be a custom form component wrapper, i think it has more usages in this case, nevertheless for form control field it works but with an additional change providing the magic tag property again but with a label because i have a form control field !
then, about the usage of QField in other components ? does this change breaks anything ?
three components inherit QField, those are (QInput, QFile, QSelect). and the problem is that they actually need a label tag!
So i change QField usages in thpse components ? I guess not, Instead that passed flag argument inside QField tagProp: true handles this for me, look:
tag: tagProp === true ? computed(() => props.tag) : { value: 'label' }
and that helps not to change anything ! becaue tagProp flag is only true within QField.
to conclude this, the bug (event triggering) should have more priority than a11y violation which can be fixed after providing the tag property therefore i have updated the documenation, i found it handy to mention the importance of tag property.
Thanks for your time <3