[LEMS-2880] Bugfix the close button focus outline on hover in keypad and other style issues #2296
Conversation
…and adjusted styling
… hover in keypad and other style issues
|
Size Change: +4 B (0%) Total Size: 735 kB
ℹ️ View Unchanged
|
npm Snapshot: PublishedGood news!! We've packaged up the latest commit from this PR (3caf2c7) and published it to npm. You Example: pnpm add @khanacademy/perseus@PR2296If you are working in Khan Academy's webapp, you can run: ./dev/tools/bump_perseus_version.sh -t PR2296 |
| opened={this.state.keypadOpen} | ||
| onClose={() => this.closeKeypad()} |
There was a problem hiding this comment.
Interesting why remove the onClose event? How was this effecting the style of the close button?
There was a problem hiding this comment.
onClose event was remove and used the showDismiss props of the keypad component. The onClose event is something that the Popover component handles which is the outer component which has a different position of the close button, but by leveraging the showDismiss props it will correctly align in the TabBar which the icons are position as part of the inner component.
…ugfix/LEMS-2880-reinstate-focus-outline
.changeset/dry-shirts-refuse.md
Outdated
| "@khanacademy/perseus": patch | ||
| --- | ||
|
|
||
| [LEMS-2880] Bugfix the close button focus outline on hover in keypad and other style issues |
There was a problem hiding this comment.
Two things:
- let's avoid JIRA ticket numbers in the changelog entries. This is an open project and JIRA is closed so not everyone will be able to reference the ticket.
- Regarding wording, what exactly was fixed with the outline? and what are the other styling issues? Basically, if I read this in the changelog of the new release, how do I know what specifically was fixed? :)
There was a problem hiding this comment.
Thanks @jeremywiebe for your comment, i'll keep that in mind moving forward 😉 I also updated the wording to properly reflect the changes I made. Please let me know if there are other changes I need.
| @@ -77,7 +77,7 @@ export function V2KeypadWithMathquill() { | |||
| content={ | |||
| <PopoverContentCore | |||
| style={{ | |||
| padding: 10, | |||
| padding: 0, | |||
There was a problem hiding this comment.
Given this is in a Story... did you also test this fix in webapp? Just want to make sure we aren't fixing this style issue for a story only.
There was a problem hiding this comment.
That's a good call! I'll do that. Base on code search we might also need to update the webapp side. Let me get back to you on this after i'm fully setup with the webapp and have a separate PR for it.
Summary:
This PR will fix the
MathInputkeypad close button's focus outline on hover. It also includes fixing the backspace keypad background color and overall padding for consistency with mobile.Issue: LEMS-2880
Test plan:
MathInputv2 Keypad With Mathquill in Storybook.Affected UI