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

fix: ModalDialog, PosterImage :focus styles #7113

Merged

Conversation

acmertz
Copy link
Contributor

@acmertz acmertz commented Mar 1, 2021

Fixes #7112

Description

Fixes a styling issue that occurred after #6871 where the background styles would be removed from ModalDialog and PosterImage components when they received focus due to a click.

Specific Changes proposed

Use :not([tabindex="-1"]) to prevent the global styles for mouse/keyboard focus on a control from applying to ModalDialog and PosterImage components' backgrounds.

Requirements Checklist

  • Feature implemented / Bug fixed
  • If necessary, more likely in a feature request than a bug fix
    • Change has been verified in an actual browser (Chrome, Firefox, IE)
    • Unit Tests updated or fixed
    • Docs/guides updated
    • Example created (starter template on JSBin)
  • Reviewed by Two Core Contributors

@gkatsev
Copy link
Member

gkatsev commented Mar 1, 2021

Thanks for the PR. While it does fix the issue it also brings back the outline which we don't want to happen. Looking into it, it seems like it's because we set background: none for these as part of #5558.
I think we potentially want to make the change in #5888 to be more specific and not unset background in all cases.

@acmertz
Copy link
Contributor Author

acmertz commented Mar 2, 2021

@gkatsev No problem. I've updated the PR - have a look and let me know if that's better.

@gkatsev
Copy link
Member

gkatsev commented Mar 9, 2021

Thanks, this is great. Targeting it specifically is helpful. I verified it, took me a second to figure out what #5888 was trying to fix.

@gkatsev gkatsev added the needs: LGTM Needs one or more additional approvals label Mar 9, 2021
Copy link
Member

@misteroneill misteroneill left a comment

Choose a reason for hiding this comment

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

Nice, thanks!

@misteroneill misteroneill removed the needs: LGTM Needs one or more additional approvals label Mar 9, 2021
@gkatsev gkatsev merged commit 1b52e7b into videojs:main Mar 9, 2021
edirub pushed a commit to edirub/video.js that referenced this pull request Jun 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ModalDialog and PosterImage components lose background styles if a non-focusable child element is clicked
3 participants