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

[Shape] Focussed Checkbox inside shape breaks animation #529

Merged
merged 3 commits into from
Mar 1, 2019

Conversation

lubber-de
Copy link
Member

@lubber-de lubber-de commented Feb 28, 2019

Description

When a shape had a focussed input inside, any animation was broken.
Previously, you had to click outside of the shape to blur the input to make the animation work again.

Although in the example below, the checkbox indeed gets blurred right before the button to animate the shape is clicked, it seems the browser engine still gets confused and does not recognize the change early enough.
This PR now delays the animation for a short time in case of input being part of the the shape, so the blurred checkout is also recognized by the browser engine at animation time

Testcase

Check/uncheck the checkbox. Then quickly (means don't hold the mousebutton for too long) click the Button to trigger the animation

Broken

https://jsfiddle.net/o5tqeuha/

Fixed

https://jsfiddle.net/xr2jvnof/

Screenshot

Broken Fixed
checkbox_shape_broken checkbox_shape_fixed

Closes

Semantic-Org/Semantic-UI#6583

@lubber-de lubber-de added type/bug Any issue which is a bug or PR which fixes a bug lang/javascript Anything involving JavaScript state/awaiting-reviews Pull requests which are waiting for reviews tag/sui-issue Taken from an existing Issue/PR of SUI labels Feb 28, 2019
@lubber-de lubber-de added this to the 2.7.x milestone Feb 28, 2019
y0hami
y0hami previously approved these changes Feb 28, 2019
Copy link
Member

@y0hami y0hami left a comment

Choose a reason for hiding this comment

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

LGTM

prudho
prudho previously approved these changes Mar 1, 2019
Copy link
Contributor

@prudho prudho left a comment

Choose a reason for hiding this comment

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

LGTM

@lubber-de
Copy link
Member Author

I had to adjust the delay for mobile, see latest commit and here
https://jsfiddle.net/cvw7p4zL/2/

@y0hami y0hami dismissed stale reviews from prudho and themself March 1, 2019 08:07

new commits

@lubber-de
Copy link
Member Author

What do you guys think? Should I remove the mobile recognition again and just stick with 150ms all the time, also on desktop then?

Copy link
Member

@y0hami y0hami left a comment

Choose a reason for hiding this comment

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

LGTM

@lubber-de
Copy link
Member Author

Removed mobile check and just increased to 150ms
New fiddle: https://jsfiddle.net/xr2jvnof/

@lubber-de lubber-de requested a review from prudho March 1, 2019 08:39
Copy link
Contributor

@prudho prudho left a comment

Choose a reason for hiding this comment

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

LGTM

@y0hami y0hami merged commit 1b4aa99 into fomantic:develop Mar 1, 2019
@y0hami y0hami removed the state/awaiting-reviews Pull requests which are waiting for reviews label Mar 1, 2019
@lubber-de lubber-de modified the milestones: 2.7.x, 2.7.3 Mar 1, 2019
@y0hami y0hami mentioned this pull request Apr 1, 2019
@lubber-de lubber-de deleted the fix/6583/shape_with_checkbox branch April 2, 2019 09:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang/javascript Anything involving JavaScript tag/sui-issue Taken from an existing Issue/PR of SUI type/bug Any issue which is a bug or PR which fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants