chore(new-nav): remove ScrollShadowWrapper usage#2551
Conversation
Use a div with max-h-[300px] in TfvarsFilesPopover instead of ScrollShadowWrapper Remove the hideSides prop and always render the top and bottom scroll shadows in ScrollShadowWrapper
|
Qovery can create a Preview Environment for this PR.
This comment has been generated from Qovery AI 🤖.
|
rmnbrd
changed the title
There was a problem hiding this comment.
Pull request overview
Updates scrolling UI behavior by simplifying the .tfvars files popover list container and adjusting ScrollShadowWrapper so its scroll shadows are always rendered (no opt-out).
Changes:
- Removed the
hideSidesprop fromScrollShadowWrapperand always render top/bottom shadow elements. - Replaced
ScrollShadowWrapperusage inTfvarsFilesPopoverwith a simpledivcapped atmax-h-[300px].
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| libs/shared/ui/src/lib/components/scroll-shadow-wrapper/scroll-shadow-wrapper.tsx | Removes hideSides prop and unconditionally renders top/bottom shadow elements. |
| libs/domains/service-settings/feature/src/lib/terraform-variables-settings/terraform-tfvars-popover/terraform-tfvars-popover.tsx | Swaps ScrollShadowWrapper for a max-height container around the reorderable tfvars file list. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| </> | ||
| ) : tfVarFiles.length > 0 ? ( | ||
| <ScrollShadowWrapper hideSides className="max-h-[300px]"> | ||
| <div className="max-h-[300px]"> |
There was a problem hiding this comment.
max-h-[300px] alone won’t make the list scrollable; with the default overflow: visible the items will overflow outside the popover instead of scrolling (previously handled by ScrollShadowWrapper’s overflow-y-auto). Add overflow-y-auto (and any needed padding like the prior pr-[1px]) to preserve the intended scroll behavior.
| <div className="max-h-[300px]"> | |
| <div className="max-h-[300px] overflow-y-auto pr-[1px]"> |
| <div className="max-h-[300px]"> | ||
| <Reorder.Group axis="y" values={tfVarFiles} onReorder={onReorder}> |
There was a problem hiding this comment.
After replacing ScrollShadowWrapper with a div, the ScrollShadowWrapper import from @qovery/shared/ui becomes unused and will fail lint/TS checks. Remove it from the imports list.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## new-navigation #2551 +/- ##
=================================================
Coverage ? 44.58%
=================================================
Files ? 1055
Lines ? 21375
Branches ? 6292
=================================================
Hits ? 9529
Misses ? 10123
Partials ? 1723
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary
Use a div with max-h-[300px] in TfvarsFilesPopover instead of ScrollShadowWrapper
Remove the hideSides prop and always render the top and bottom scroll shadows
in ScrollShadowWrapper