ErrorFormTooltip accessibility #4652
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Storybook staging is available at https://kiwicom-orbit-domi-error-tooltip-a11y.surge.sh Playroom staging is available at https://kiwicom-orbit-domi-error-tooltip-a11y.surge.sh/playroom |
|
Size Change: -276 B (-0.06%) Total Size: 463 kB
ℹ️ View Unchanged
|
Deploying orbit with
|
| Latest commit: |
805f7fe
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ee036703.orbit.pages.dev |
| Branch Preview URL: | https://domi-error-tooltip-a11y.orbit.pages.dev |
domihustinova
force-pushed
the
domi/error-tooltip-a11y
branch
from
77de0f4 to
c026478
Compare
![callstackai[bot]](https://avatars.githubusercontent.com/in/1040204?s=60&v=4){kind=small}
There was a problem hiding this comment.
Key Issues
The lack of error handling and validation props in InputGroup components could lead to incomplete error state management and accessibility issues. The ariaHidden prop defaults to hiding the chevron icon from screen readers, potentially affecting accessibility if the icon indicates collapse state. Changing label type from Common.Translation to string may cause type errors and break existing code relying on translation objects.
domihustinova
commented
Mar 7, 2025
domihustinova
force-pushed
the
domi/error-tooltip-a11y
branch
from
9e89908 to
6a178ea
Compare
DSil
reviewed
Mar 11, 2025
...eld/InputField.ct.tsx-snapshots/visual-InputField-screenshot-help-focus-1-Desktop-darwin.png
Outdated
Show resolved
Hide resolved
docs/src/documentation/05-development/04-migration-guides/01-v22.mdx
Outdated
Show resolved
Hide resolved
domihustinova
force-pushed
the
domi/error-tooltip-a11y
branch
from
6a178ea to
8cdbeb6
Compare
LLM Analysis of PR ChangesPull Request Analysis - /pull/4652 - ErrorFormTooltip accessibility1. Summary of the changes: This pull request focuses on improving the accessibility of the The key changes are:
2. Potential issues or bugs you notice:
3. Suggestions for improvements:
4. Any security concerns:
5. Code quality observations:
Overall Feedback: This pull request seems to be a step in the right direction for improving the accessibility and consistency of the Orbit component library by simplifying the However, the version downgrades are a significant red flag and must be investigated and clarified immediately. Additionally, ensure you thoroughly validate the accessibility improvements, provide clear documentation for the breaking change, and justify the icon deletion. Address the suggestions for improvements provided above to enhance the quality and clarity of this PR before merging. |
DSil
reviewed
Mar 12, 2025
BREAKING CHANGE: The ErrorFormTooltip no longer renders the Close button as tooltip can not contain interactive elements.
domihustinova
force-pushed
the
domi/error-tooltip-a11y
branch
from
8cdbeb6 to
59efd48
Compare
LLM Analysis of PR ChangesOkay, I've analyzed the provided Git diff. Here's my feedback: Pull Request Feedback: /pull/4652 - ErrorFormTooltip accessibility1. Summary of ChangesThis pull request primarily focuses on improving the accessibility and simplifying the API of the Essentially, this change enforces a consistent behavior for help tooltips: they are now always closable by clicking outside the tooltip or when the associated input element loses focus. The explicit "close" button and the Alongside the code modifications, the pull request includes:
2. Potential Issues or Bugs
3. Suggestions for Improvements
4. Security Concerns
5. Code Quality Observations
Overall: This pull request appears to be a well-thought-out and executed change that improves the accessibility and simplifies the API of the Recommendations:
After addressing these minor suggestions, the pull request looks good to be merged. |
Comment on lines
75
to
+76
| aria-live="polite" | ||
| aria-hidden={!shown} |
There was a problem hiding this comment.
🐛 Possible Bug
Setting aria-hidden={!shown} while maintaining aria-live="polite" is contradictory. When the tooltip is hidden, screen readers won't announce changes due to aria-hidden, making aria-live ineffective.
Suggested change
| aria-live="polite" | |
| aria-hidden={!shown} | |
| aria-live="polite" |
BREAKING CHANGE: The InputField component no longer accepts the helpClosable prop, since the help tooltip no longer renders the Close button.
BREAKING CHANGE: The InputFile component no longer accepts the helpClosable prop, since the help tooltip no longer renders the Close button.
BREAKING CHANGE: The InputGroup component no longer accepts the helpClosable prop, since the help tooltip no longer renders the Close button.
BREAKING CHANGE: The InputSelect component no longer accepts the helpClosable prop, since the help tooltip no longer renders the Close button.
BREAKING CHANGE: The Select component no longer accepts the helpClosable prop, since the help tooltip no longer renders the Close button.
BREAKING CHANGE: The Textarea component no longer accepts the helpClosable prop, since the help tooltip no longer renders the Close button.
domihustinova
force-pushed
the
domi/error-tooltip-a11y
branch
from
59efd48 to
805f7fe
Compare
LLM Analysis of PR ChangesOkay, I've analyzed the provided Git diff from pull request /pull/4652. Here's my feedback: Pull Request Analysis - Orbit #4652: ErrorFormTooltip accessibility1. Summary of ChangesThis pull request primarily focuses on improving the accessibility of the
2. Potential Issues or Bugs
3. Suggestions for Improvements
4. Security Concerns
5. Code Quality Observations
Overall Feedback: This pull request seems to be a positive step towards improving the accessibility and consistency of the Actionable Feedback Summary:
By addressing these points, the pull request can be further strengthened and ensure a smooth and positive experience for users migrating to Orbit v22. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part of https://kiwicom.atlassian.net/browse/FEPLT-2247
✨
Description by Callstackai
This PR updates the
ErrorFormTooltipcomponent by removing thehelpClosableprop and adjusting related components accordingly. It also includes updates to documentation and tests reflecting these changes.Diagrams of code changes
sequenceDiagram participant User participant FormComponent participant ErrorFormTooltip participant Tooltip Note over FormComponent: Components affected:<br/>InputField, InputFile, InputGroup,<br/>InputSelect, Select, Textarea User->>FormComponent: Interacts with form component FormComponent->>ErrorFormTooltip: Shows help/error tooltip alt Help tooltip shown ErrorFormTooltip->>Tooltip: isHelp=true, shown=true Note over Tooltip: Tooltip closes on:<br/>1. Outside click<br/>2. Input blur else Error tooltip shown ErrorFormTooltip->>Tooltip: shown=true Note over Tooltip: Tooltip displays error message end Note over ErrorFormTooltip: Removed helpClosable prop<br/>Simplified tooltip behavior User->>FormComponent: Clicks outside or blurs input FormComponent->>ErrorFormTooltip: onShown(false) ErrorFormTooltip->>Tooltip: Hide tooltipFiles Changed
helpClosableprop.helpClosableprop fromErrorFormTooltipand adjusted related logic.helpClosableprop.helpClosablefrom the props interface.helpClosablefrom story definitions.helpClosablefrom the InputField story.helpClosablefrom InputFile story.helpClosablefrom InputGroup story.helpClosablefrom InputSelect story.helpClosablefrom Textarea story.This PR includes files in programming languages that we currently do not support. We have not reviewed files with the extensions
.mdx,.md. See list of supported languages.