Clay: Checkbox, Radio and Toggle components of Clay lose focus and get it back when they're clicked

Created on 28 Feb 2019  路  18Comments  路  Source: liferay/clay

From JIRA:

Steps to reproduce

Go to https://clayui.com/docs/components/forms/radio-check-toggle.html.
Add focus to one of the following components: Checkbox, Radio and Toggle.
Click on one of them without release the mouse button.
Check the focus is lost.
Release the mouse button and the focus is back to the clicked component.

This issue is causing an error in another components like ClayDropdown using radio, when we listen to the component itemClicked event the event is emitted twice.

A temporary solution proposed by @matuzalemsteles was change the input type radio, and checkbox z-index to value 2.

@NemethNorbert @natocesarrego @matuzalemsteles

2.x clay-components clay-css next-release bug

Most helpful comment

Just an update on this, the double event when clicked on the checkbox... are resolved with the z-index change but we still have two events when clicked on the checkbox text, this seems to be an old problem due to our markup is wrapped inside a <label />, needs to be investigated what is the best solution for this. Since if we remove <label /> we will solve these two problems without having to change z-index.

All 18 comments

Just an update on this, the double event when clicked on the checkbox... are resolved with the z-index change but we still have two events when clicked on the checkbox text, this seems to be an old problem due to our markup is wrapped inside a <label />, needs to be investigated what is the best solution for this. Since if we remove <label /> we will solve these two problems without having to change z-index.

By the way, @matuzalemsteles, I realized that the z-index solution doesn't work when the mentioned components are inline, therefore would be necessary check this use case too.

Best.

hey @pat270 can we have a markup change that we could help with this?

@natocesarrego can you give me a specific use case for changing display to inline on a custom control? It wasn't designed to have the display property changed. There is a modifier class, custom-control-inline you can add to custom-control that will render checkboxes/radios inline with proper spacing defined by Lexicon.

If we really need this use case I can tweak the CSS properties to make it work.

Hi @pat270,

First of all, thanks for your explanation. In light of this, I realized that the specific issue related to the inline setting is happening on the Portlet's side only and not on Clay's side.

Best.

Hey @pat270, do you have any updates about the possible markup breaking to solve this issue?

/cc @NemethNorbert

Just an update on this, the double event when clicked on the checkbox... are resolved with the z-index change but we still have two events when clicked on the checkbox text, this seems to be an old problem due to our markup is wrapped inside a <label />, needs to be investigated what is the best solution for this. Since if we remove <label /> we will solve these two problems without having to change z-index.

@diegonvs I'm not sure if I understand the problem. If the checkbox or radio is wrapped inside label and you click on the text quickly the change event gets fired twice by the browser?

The z-index thing fixed an issue where the real checkbox (hidden from view with opacity: 0; position: absolute;) was under the label which is why it lost focus when moving the mouse away mid click (the focus was actually on the label). Even with the z-index fix this still happens when you click on the text and move the mouse away mid click.

If you don't want everything wrapped inside a label the markup is:

<div class="custom-control custom-checkbox">
    <input class="custom-control-input" id="theInputId" type="checkbox">
    <label class="custom-control-label" for="theInputId">
        <span class="custom-control-label-text">Unchecked</span>
    </label>
</div>

Thanks a lot for the discussion and the resolution of this issue @diegonvs, @matuzalemsteles and @pat270! 馃槂

I just have one more question: how this fix will be handled on Jira side? I filled out the LPS-91383 reporting this issue.

Thanks in advance.

hey @natocesarrego, sorry for the confusion only the problem with z-index in CSS has been fixed and is already included in the last version of Clay (2.10.0), this issue will be closed once we update the markup provided by Patrick above in ClayCheckbox and ClayRadio to match, I'm working on it.

On LPS, we have to do the backporting of that fix for 7.1 too, @pat270 do you know how we can proceed with this fix for there?

Thanks for the update, @matuzalemsteles. 馃槈

@matuzalemsteles We can add the z-index fix in the theme or update clay css to 2.10.0?

hey @pat270, I do not know if it will cause problems with css in 7.1, I think we've had some visual changes that can cause regression to 7.1 updating CSS to 2.10.0

@matuzalemsteles I think the only visual change I can think of are the inverted sticker colors, which might be ok to break since it has to do with accessibility? I can test it on Monday and see what breaks, otherwise we can just add it in styled theme.

@matuzalemsteles I tested in 7.1 and I forgot the Lexicon team changed the font sizes on h1 - h6 elements to be bigger. I think this is another breaking change that is pretty big. The z-index fix should go in the styled theme.

Oh yeah, thanks @pat270, can you take care of updating the theme?

@matuzalemsteles @natocesarrego The backport is at https://github.com/jonmak08/liferay-portal/pull/2059.

Also updating the markup in Clay Checkbox / Radio won't fix anything. The problem is the way the Forms Team is validating the checkbox/radio. It looks like it's being validated on blur for a single input. The error message still shows briefly when you decide to click any radio input. The requirement to run the validator needs to be better.

hey @pat270, on the question markup in Clay, it will solve the problem of two events, so I think we should correct here also in the components of Clay. I'm going to work on it this afternoon.

It seems that this problem is also reproducible on a toggle switch, this is because the input is in the upper corner, if you click exactly on it, it is not playable but clicking off will happen. @natocesarrego suggested that perhaps increasing the width of the input could cover the case and adding the z-index as well.

Hi @matuzalemsteles and @pat270,

I did a quick solution, which was intended to be a work around to the customer, while he waits the final solution. Basically, I just increased the size (width and height) of the checkbox to make sure the checkbox will be clicked when the toggle element is clicked, please see here.

However, my solution isn't the best, once it's not handling the cases in which the browser window is resized, therefore I understand that a final solution should handle the browser window resize.

That's all from my side for now guys.

Thanks for your support!

Was this page helpful?
0 / 5 - 0 ratings