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
feat: migrate challenge modals #54179
feat: migrate challenge modals #54179
Conversation
This reverts commit dac787d.
The The I'm confident that our implementation is correct because:
|
630b608
to
94a3f19
Compare
40e25c2
to
406d31e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Sembauke I made some changes to the PR as I tested the changes locally.
The changes are:
- Handling the
onKeyDown
event incompletion-modal
- This also includes adding the
onKeyDown
prop to the Modal component)
- This also includes adding the
- Restoring the
!canFocusEditor
condition inhotkeys.tsx
(f3adb37#diff-0f97911cdf2b391a4029ce7d981cdd922bd45c140c8a7afd55282c79bc04f959R204)- I think there is a bug in the focus management that prevents us from being able to trigger the shortcuts modal consistently. It's probably related to the last point in Most common incorrect answers for step-1 #54024 (comment), so I think we should look into the issue separately (and add more tests to cover the expected behaviors)
- Adding
closeButtonClassnames='close'
toModal.Header
so that the close button is displayed properly- There are some CSS rules in /learn that targets the
button
element, which overridesui-components
styles. And thus, we need to pass theclose
class name from /learn, which is tied with some CSS styles, to compete the CSS specificity
- There are some CSS rules in /learn that targets the
- Adding
xLarge
Modal size to accommodate for thevideo-modal
- Updating the modal padding value to accommodate for small screen sizes
Co-authored-by: Huyen Nguyen <25715018+huyenltnguyen@users.noreply.github.com>
Checklist:
main
branch of freeCodeCamp.ref #52759