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(input): add focus ring to the clear input button for ionic theme #29409
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@@ -214,7 +212,7 @@ configs({ modes: ['ionic-md'], directions: ['ltr'] }).forEach(({ title, config } | |||
await expect(clearButton).toBeVisible(); | |||
|
|||
// ensure blurring native input doesn't immediately hide clear button | |||
await page.keyboard.press('Tab'); | |||
await pageUtils.pressKeys('Tab'); |
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.
Made this change to be consistent and to use the Tab logic for all OSs.
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.
Do we have screenshot coverage for the focus ring on the clear button for the ionic theme?
const clearButton = input.locator('.input-clear-icon'); | ||
clearButton.evaluate((el: HTMLElement) => el.classList.add('ion-focused')); | ||
await page.waitForChanges(); | ||
await pageUtils.pressKeys('Tab'); | ||
|
||
const container = page.locator('#container'); | ||
await expect(container).toHaveScreenshot(screenshot(`input-clear-button-focused`)); |
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.
How are these screenshots not updated by this PR? https://github.com/ionic-team/ionic-framework/blob/ROU-4848/core/src/components/input/test/basic/input.e2e.ts-snapshots/input-clear-button-focused-ionic-md-ltr-light-Mobile-Chrome-linux.png
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.
The screenshots were not updated because no visual changes happened. The focus ring is still the same shape, size, color, etc.
Issue number: internal
What is the current behavior?
When a user tabs into the clear icon button within
ion-input
, it does not display a focus ring. This was displaying prior but this PR caused it to stop displaying without realizing it.What is the new behavior?
Does this introduce a breaking change?
Other information
This is for the
ionic
theme.