Chore(UI): New service flow feedback#29697
Open
aniketkatkar97 wants to merge 5 commits into
Open
Conversation
aniketkatkar97
requested review from
a team,
chirag-madlani and
karanh37
as code owners
github-actions
Bot
added
safe to test
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses UI consistency feedback for the “new service flow” by aligning the service tabs badge sizing with the design system and by refactoring the Navigation Guard modal to use the shared modal footer component.
Changes:
- Add a tab-size → badge-size mapping so tab badges render at a smaller, design-system-aligned size.
- Update the Tab badge sizing + conditional styling to use the new mapping.
- Refactor
NavigationGuardModalactions to useDialog.Footerinstead of a custom wrapper.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| openmetadata-ui/src/main/resources/ui/src/components/common/NavigationGuardModal/NavigationGuardModal.tsx | Switches the navigation guard modal action area to use Dialog.Footer for shared modal structure/styling. |
| openmetadata-ui-core-components/src/main/resources/ui/src/components/application/tabs/tabs.tsx | Introduces a size mapping to ensure badges are smaller than their associated tab size and updates the Tab badge rendering accordingly. |
Contributor
Jest test CoverageUI tests summary
|
Contributor
🔴 Playwright Results — 1 failure(s), 26 flaky✅ 4479 passed · ❌ 1 failed · 🟡 26 flaky · ⏭️ 37 skipped
Genuine Failures (failed on all attempts)❌
|
…ion blocker modal is open
Code Review ✅ ApprovedImproves UI consistency by centralizing tab badge sizing, refactoring NavigationGuardModal with semantic Dialog.Footer components, and adjusting resizable panel z-index to prevent modal overlap. Comprehensive test coverage added for all modal interaction paths; no issues found. OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
|
This file contains hidden or 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
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.
Fixes #29696
This pull request makes improvements to the tab and modal components to enhance UI consistency and code maintainability. The main changes are focused on adjusting badge sizing in tabs to match the design system and refactoring the modal button layout for better structure.
Tab component improvements:
badgeSizeMappingconstant to ensure that badge sizes are always smaller than their corresponding tab sizes, following the design system guidelines. (openmetadata-ui-core-components/src/main/resources/ui/src/components/application/tabs/tabs.tsx, openmetadata-ui-core-components/src/main/resources/ui/src/components/application/tabs/tabs.tsxR109-R115)Tabcomponent to use the mapped badge size and adjust conditional styling accordingly. (openmetadata-ui-core-components/src/main/resources/ui/src/components/application/tabs/tabs.tsx, openmetadata-ui-core-components/src/main/resources/ui/src/components/application/tabs/tabs.tsxL216-R226)Sizesfor proper type safety with the new badge size mapping. (openmetadata-ui-core-components/src/main/resources/ui/src/components/application/tabs/tabs.tsx, openmetadata-ui-core-components/src/main/resources/ui/src/components/application/tabs/tabs.tsxL1-R1)Modal component refactor:
NavigationGuardModalbutton layout to useDialog.Footerinstead of a custom<div>, improving semantic structure and maintainability. (openmetadata-ui/src/main/resources/ui/src/components/common/NavigationGuardModal/NavigationGuardModal.tsx, openmetadata-ui/src/main/resources/ui/src/components/common/NavigationGuardModal/NavigationGuardModal.tsxL58-R65)Summary by Gitar
z-indexofresizable-panelssplitter to50to prevent UI overlay issues when the modal is open.onClose={onStay}toNavigationGuardModalfor consistent dialog behavior.Dialog.FooterinNavigationGuardModalwith extra top margin.NavigationGuardModal.test.tsxto ensure proper modal rendering and action handling foronLeaveandonStaycallbacks.This will update automatically on new commits.
Greptile Summary
This PR makes two focused UI improvements: badge sizing in the
Tabcomponent now follows a design-system rule (badge is always one size smaller than its tab), and theNavigationGuardModalclose button is given explicit semantic meaning viaonClose={onStay}.tabs.tsx: AbadgeSizeMappingconstant (sm→xs,md→sm) replaces the directsizepass-through toBadge, along with a corrected conditional margin class that now fires onmd-mapped badges rather thansmtabs.NavigationGuardModal.tsx:onClose={onStay}is passed toDialogso the X button consistently triggers the "stay" path;tw:sm:mt-3adjusts footer spacing at thesmbreakpoint. TheDialogimplementation usesonClose ?? close, soonClosefully replaces the built-in dismiss — the parent remains responsible for settingisOpen=false.NavigationGuardModal.test.tsx: New test suite covering closed-state rendering, open-state content, and isolatedonLeave/onStaycallback assertions.Confidence Score: 5/5
Safe to merge — all three changed files contain narrow, intentional adjustments with no correctness concerns.
The badge-size mapping is well-typed and exhaustively covers all existing tab sizes. The NavigationGuardModal changes correctly leverage the Dialog component's onClose-replaces-close contract (confirmed by reading modal.tsx), and the new tests cover the key interaction paths.
No files require special attention.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[User triggers navigation away] --> B[NavigationGuardModal opens\nisOpen=true] B --> C{User action} C -->|Clicks Discard button| D[onLeave called\nParent navigates away\nisOpen=false] C -->|Clicks Continue Editing button| E[onStay called\nParent cancels navigation\nisOpen=false] C -->|Clicks X close button| F["onClose={onStay} fires\nonStay called\nParent cancels navigation\nisOpen=false"] C -->|Clicks ModalOverlay backdrop| G["onOpenChange false fires\nonStay called\nParent cancels navigation\nisOpen=false"] D --> H[Modal closes] E --> H F --> H G --> H%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% flowchart TD A[User triggers navigation away] --> B[NavigationGuardModal opens\nisOpen=true] B --> C{User action} C -->|Clicks Discard button| D[onLeave called\nParent navigates away\nisOpen=false] C -->|Clicks Continue Editing button| E[onStay called\nParent cancels navigation\nisOpen=false] C -->|Clicks X close button| F["onClose={onStay} fires\nonStay called\nParent cancels navigation\nisOpen=false"] C -->|Clicks ModalOverlay backdrop| G["onOpenChange false fires\nonStay called\nParent cancels navigation\nisOpen=false"] D --> H[Modal closes] E --> H F --> H G --> HReviews (5): Last reviewed commit: "Merge branch "main" into "fix-navigation..." | Re-trigger Greptile