Skip to content

Use :focus-within for .form-file focus state#29036

Merged
XhmikosR merged 6 commits intomasterfrom
form-file-focus
Jul 17, 2019
Merged

Use :focus-within for .form-file focus state#29036
XhmikosR merged 6 commits intomasterfrom
form-file-focus

Conversation

@mdo
Copy link
Copy Markdown
Member

@mdo mdo commented Jul 13, 2019

Fixes #26563, closes #26576, closes #29021.

@mdo mdo requested a review from a team as a code owner July 13, 2019 20:51
@mdo mdo force-pushed the form-file-focus branch from 7397a85 to 2cc53ac Compare July 13, 2019 20:53
@patrickhlauke
Copy link
Copy Markdown
Member

Note that this change, while it fixes Firefox, now breaks the focus indication in current Edge

@mdo
Copy link
Copy Markdown
Member Author

mdo commented Jul 13, 2019

Note that this change, while it fixes Firefox, now breaks the focus indication in current Edge

Does that mean we need to have both selectors then?

@patrickhlauke
Copy link
Copy Markdown
Member

Probably yes

@mdo
Copy link
Copy Markdown
Member Author

mdo commented Jul 15, 2019

With both accounted for, looks good in my quick test. Can anyone confirm?

Copy link
Copy Markdown
Member

@MartijnCuppens MartijnCuppens left a comment

Choose a reason for hiding this comment

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

:focus-within ain't supported in Edge & IE so the rule will be ignored. We 'll need to duplicate the block to make this work (same reason we needed to duplicate the blocks of the custom range theming,https://github.com/twbs/bootstrap/blob/master/scss/forms/_form-range.scss)

@patrickhlauke
Copy link
Copy Markdown
Member

:focus-within ain't supported in Edge & IE so the rule will be ignored. We 'll need to duplicate the block to make this work (same reason we needed to duplicate the blocks of the custom range theming,https://github.com/twbs/bootstrap/blob/master/scss/forms/_form-range.scss)

yup, and just checking with Edge there's still no focus indication. and now in IE11 it only shows the caret/cursor in the first part of the control (where the filename is) and no glowing blue outline (which does show in current v4 in IE11).

so yes, this needs to maintain the v4 styles, and then add the :focus-within as a separate replicated block.

@patrickhlauke
Copy link
Copy Markdown
Member

confirming that the file inputs in https://deploy-preview-29036--twbs-bootstrap.netlify.com/docs/4.3/forms/file/ now show the appropriate focus indicator in IE11, Edge, Firefox and Chrome 👍

Copy link
Copy Markdown
Contributor

@ysds ysds left a comment

Choose a reason for hiding this comment

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

Looks good!

@XhmikosR XhmikosR merged commit 99676c7 into master Jul 17, 2019
@XhmikosR XhmikosR deleted the form-file-focus branch July 17, 2019 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Custom file input has no focus indication in Firefox

5 participants