Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Core: Minor UI fixes#33218

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
ghengeveld wants to merge2 commits intonext
base:next
Choose a base branch
Loading
fromminor-ui-fixes
Open

Core: Minor UI fixes#33218

ghengeveld wants to merge2 commits intonextfromminor-ui-fixes

Conversation

@ghengeveld
Copy link
Member

@ghengeveldghengeveld commentedNov 28, 2025
edited by coderabbitaibot
Loading

Closes #

What I did

  • Add missingariaLabel props to fix console warnings.
  • Prevent ugly default outline from showing up on tooltips and popovers when using keyboard trigger.

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

  • stories
  • unit tests
  • integration tests
  • end-to-end tests

Manual testing

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

Documentation

  • Add or update documentation reflecting your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Checklist for Maintainers

  • When this PR is ready for testing, make sure to addci:normal,ci:merged orci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.ts

  • Make sure this PR containsone of the labels below:

    Available labels
    • bug: Internal changes that fixes incorrect behavior.
    • maintenance: User-facing maintenance tasks.
    • dependencies: Upgrading (sometimes downgrading) dependencies.
    • build: Internal-facing build tooling & test updates. Will not show up in release changelog.
    • cleanup: Minor cleanup style change. Will not show up in release changelog.
    • documentation: Documentationonly changes. Will not show up in release changelog.
    • feature request: Introducing a new feature.
    • BREAKING CHANGE: Changes that break compatibility in some way with current major version.
    • other: Changes that don't fit in the above categories.

🦋 Canary release

This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the@storybookjs/core team here.

core team members can create a canary releasehere or locally withgh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>

Summary by CodeRabbit

Style

  • Enhanced outline styling in popover and tooltip UI components to use optimized CSS properties, resulting in improved visual consistency and better browser compatibility across different focus states and interactions.

Accessibility

  • Strengthened accessibility support in the tag filter panel by updating ARIA label attributes on interactive UI elements, providing better screen reader compatibility and enhanced support for assistive technology users.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

📝 Walkthrough

Walkthrough

Three component files receive styling and accessibility attribute updates. Two files (Popover and Tooltip providers) replace CSS outline handling from transparent color to explicit none property. One file adds aria labels to action list components. No control flow or behavioral changes.

Changes

Cohort / File(s)Summary
Outline CSS style updates
code/core/src/components/components/Popover/PopoverProvider.tsx,code/core/src/components/components/tooltip/TooltipProvider.tsx
ReplacedoutlineColor: 'transparent' withoutline: 'none' in inline styles to suppress focus outlines using a different CSS property approach
Accessibility attributes
code/core/src/manager/components/sidebar/TagsFilterPanel.tsx
AddedariaLabel={false} to two ActionList components (Action element in renderLink and Link element)

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • All changes are isolated, non-behavioral modifications (CSS property swaps and accessibility attribute additions)
  • No logic, data flow, or control-flow alterations
  • Straightforward cosmetic and attribute updates with minimal reasoning required per file
✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and betweenc39b35d and0461680.

📒 Files selected for processing (3)
  • code/core/src/components/components/Popover/PopoverProvider.tsx (1 hunks)
  • code/core/src/components/components/tooltip/TooltipProvider.tsx (1 hunks)
  • code/core/src/manager/components/sidebar/TagsFilterPanel.tsx (2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use camelCase for variable and function names

Files:

  • code/core/src/manager/components/sidebar/TagsFilterPanel.tsx
  • code/core/src/components/components/Popover/PopoverProvider.tsx
  • code/core/src/components/components/tooltip/TooltipProvider.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{ts,tsx}: Enable TypeScript strict mode
Export functions from modules for testing purposes

Files:

  • code/core/src/manager/components/sidebar/TagsFilterPanel.tsx
  • code/core/src/components/components/Popover/PopoverProvider.tsx
  • code/core/src/components/components/tooltip/TooltipProvider.tsx
**/*.{ts,tsx,js,jsx,json,html,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{ts,tsx,js,jsx,json,html,mjs}: Use ESLint and Prettier for code style enforcement
Run 'yarn prettier --write ' to format code after making changes
Run 'yarn lint:js:cmd ' to check for ESLint issues after making changes

Files:

  • code/core/src/manager/components/sidebar/TagsFilterPanel.tsx
  • code/core/src/components/components/Popover/PopoverProvider.tsx
  • code/core/src/components/components/tooltip/TooltipProvider.tsx
code/**/!(*.test).{ts,tsx,js,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

code/**/!(*.test).{ts,tsx,js,mjs}: Use 'logger' from 'storybook/internal/node-logger' for server-side (Node.js) logging, not console.log/console.warn/console.error
Use 'logger' from 'storybook/internal/client-logger' for client-side (browser) logging, not console.log/console.warn/console.error
Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size

Files:

  • code/core/src/manager/components/sidebar/TagsFilterPanel.tsx
  • code/core/src/components/components/Popover/PopoverProvider.tsx
  • code/core/src/components/components/tooltip/TooltipProvider.tsx
🧠 Learnings (5)
📓 Common learnings
Learnt from: SidnioulzRepo: storybookjs/storybook PR: 32458File: code/core/src/components/components/Tabs/Tabs.stories.tsx:222-227Timestamp: 2025-11-05T09:36:55.944ZLearning: Repo: storybookjs/storybook PR: 32458 — In code/core/src/components/components/Button/Button.tsx (React/TypeScript), ButtonProps includes ariaLabel?: string | false and the component maps it to the DOM aria-label. Convention: ariaLabel is mandatory on all Button usages — provide a descriptive string for icon-only buttons; set ariaLabel=false when the button’s children already serve as the accessible name. Do not suggest using a raw aria-label prop on Button call sites.
Learnt from: SidnioulzRepo: storybookjs/storybook PR: 32458File: code/core/src/manager/components/preview/Toolbar.tsx:102-105Timestamp: 2025-10-03T07:55:42.639ZLearning: In code/core/src/manager/components/preview/Toolbar.tsx, we intentionally do not add aria-label/aria-labelledby to StyledToolbar (AbstractToolbar) because the enclosing section is already labeled via an sr-only heading and the toolbar is the only content. Revisit only if real user testing indicates a need.
Learnt from: SidnioulzRepo: storybookjs/storybook PR: 32458File: code/core/src/components/components/Select/Select.tsx:200-204Timestamp: 2025-11-05T09:38:47.712ZLearning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Learnt from: SidnioulzRepo: storybookjs/storybook PR: 32458File: code/core/src/components/components/tooltip/WithTooltip.tsx:54-96Timestamp: 2025-11-05T09:37:25.920ZLearning: Repo: storybookjs/storybook — In code/core/src/components/components/tooltip/WithTooltip.tsx, the legacy WithTooltip implementation is intentionally reintroduced for backward compatibility and is deprecated; maintainers (per Sidnioulz) do not want maintenance or improvements on it. Prefer WithTooltipNew/Popover; avoid suggesting changes to WithTooltip.* going forward.
Learnt from: SidnioulzRepo: storybookjs/storybook PR: 33140File: code/core/src/manager/components/sidebar/TagsFilter.tsx:247-259Timestamp: 2025-11-25T11:09:33.798ZLearning: In the storybookjs/storybook repository, PopoverProvider creates popovers with a dialog role, so using aria-haspopup="dialog" on buttons that trigger PopoverProvider is semantically correct.
Learnt from: SidnioulzRepo: storybookjs/storybook PR: 32594File: code/core/src/components/components/Popover/WithPopover.tsx:7-9Timestamp: 2025-10-01T15:24:01.060ZLearning: In the Storybook repository, "react-aria-components/patched-dist/*" (e.g., "react-aria-components/patched-dist/Dialog", "react-aria-components/patched-dist/Popover", "react-aria-components/patched-dist/Tooltip") are valid import paths created by a patch applied to the react-aria-components package. These imports should not be flagged as broken or invalid until a maintainer explicitly states they are no longer acceptable.
📚 Learning: 2025-11-05T09:36:55.944Z
Learnt from: SidnioulzRepo: storybookjs/storybook PR: 32458File: code/core/src/components/components/Tabs/Tabs.stories.tsx:222-227Timestamp: 2025-11-05T09:36:55.944ZLearning: Repo: storybookjs/storybook PR: 32458 — In code/core/src/components/components/Button/Button.tsx (React/TypeScript), ButtonProps includes ariaLabel?: string | false and the component maps it to the DOM aria-label. Convention: ariaLabel is mandatory on all Button usages — provide a descriptive string for icon-only buttons; set ariaLabel=false when the button’s children already serve as the accessible name. Do not suggest using a raw aria-label prop on Button call sites.

Applied to files:

  • code/core/src/manager/components/sidebar/TagsFilterPanel.tsx
📚 Learning: 2025-10-03T07:55:42.639Z
Learnt from: SidnioulzRepo: storybookjs/storybook PR: 32458File: code/core/src/manager/components/preview/Toolbar.tsx:102-105Timestamp: 2025-10-03T07:55:42.639ZLearning: In code/core/src/manager/components/preview/Toolbar.tsx, we intentionally do not add aria-label/aria-labelledby to StyledToolbar (AbstractToolbar) because the enclosing section is already labeled via an sr-only heading and the toolbar is the only content. Revisit only if real user testing indicates a need.

Applied to files:

  • code/core/src/manager/components/sidebar/TagsFilterPanel.tsx
📚 Learning: 2025-11-25T11:09:33.798Z
Learnt from: SidnioulzRepo: storybookjs/storybook PR: 33140File: code/core/src/manager/components/sidebar/TagsFilter.tsx:247-259Timestamp: 2025-11-25T11:09:33.798ZLearning: In the storybookjs/storybook repository, PopoverProvider creates popovers with a dialog role, so using aria-haspopup="dialog" on buttons that trigger PopoverProvider is semantically correct.

Applied to files:

  • code/core/src/components/components/Popover/PopoverProvider.tsx
📚 Learning: 2025-11-05T09:37:25.920Z
Learnt from: SidnioulzRepo: storybookjs/storybook PR: 32458File: code/core/src/components/components/tooltip/WithTooltip.tsx:54-96Timestamp: 2025-11-05T09:37:25.920ZLearning: Repo: storybookjs/storybook — In code/core/src/components/components/tooltip/WithTooltip.tsx, the legacy WithTooltip implementation is intentionally reintroduced for backward compatibility and is deprecated; maintainers (per Sidnioulz) do not want maintenance or improvements on it. Prefer WithTooltipNew/Popover; avoid suggesting changes to WithTooltip.* going forward.

Applied to files:

  • code/core/src/components/components/Popover/PopoverProvider.tsx
  • code/core/src/components/components/tooltip/TooltipProvider.tsx
🧬 Code graph analysis (1)
code/core/src/manager/components/sidebar/TagsFilterPanel.tsx (1)
code/core/src/components/components/ActionList/ActionList.tsx (1)
  • ActionList (178-199)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: normal
  • GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (4)
code/core/src/components/components/tooltip/TooltipProvider.tsx (1)

99-99:LGTM - Correct approach to prevent focus outlines on tooltips.

Usingoutline: 'none' is more robust thanoutlineColor: 'transparent' as it completely removes the outline rather than just making it invisible. This correctly prevents the default keyboard-focus outline from appearing on the tooltip overlay.

code/core/src/manager/components/sidebar/TagsFilterPanel.tsx (2)

108-108:Correct use ofariaLabel={false} for accessible labeling.

TheActionList.Action wraps a checkbox that already has an explicitaria-label={toggleLabel} (line 115), and the visible text content provides additional context. SettingariaLabel={false} correctly signals that the children serve as the accessible name. Based on learnings, this follows the repo convention.


203-217:Correct use ofariaLabel={false} for the documentation link.

The link contains visible text "Learn how to add tags" which serves as the accessible name. SettingariaLabel={false} correctly indicates this per the repo convention. Based on learnings, this is the appropriate pattern when button/link children already provide the accessible name.

code/core/src/components/components/Popover/PopoverProvider.tsx (1)

90-90:LGTM - Consistent outline fix with TooltipProvider.

Usingoutline: 'none' correctly prevents the default keyboard-focus outline on the popover container. This is consistent with the identical change inTooltipProvider.tsx and properly addresses the PR objective.

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates thehigh-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using thehigh_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Usehigh_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.


Comment@coderabbitai help to get the list of available commands and usage tips.

@nx-cloud
Copy link

nx-cloudbot commentedNov 28, 2025
edited
Loading

View yourCI Pipeline Execution ↗ for commit0461680

CommandStatusDurationResult
nx run-many -t build --parallel=3✅ Succeeded44sView ↗

☁️Nx Cloud last updated this comment at2025-11-28 11:56:26 UTC

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@yannbfyannbfAwaiting requested review from yannbf

At least 1 approving review is required to merge this pull request.

Assignees

@ghengeveldghengeveld

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ghengeveld

[8]ページ先頭

©2009-2025 Movatter.jp