fix: Restore consistent horizontal page spacing BED-9094 - #3074
Conversation
…ding on screens < XL
…eWithTitle. Not every PageWithTitle lives under the Administration container, so putting the gutter in the one shared place for all the pages allows for consistency
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughUpdates shared, Administration, and Swagger UI layout classes. Adds tests for default and ChangesUI layout updates
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Restores consistent responsive horizontal spacing across pages by relying on MUI Container’s default gutters in PageWithTitle, while preventing doubled gutters in the Administration route container.
Changes:
- Removed Tailwind horizontal padding overrides from
PageWithTitleso MUI’s responsive gutters apply at all viewport widths (includingfullWidth). - Disabled gutters on the Administration routing
Containerand replaced the outer MUIBoxwrapper with a nativediv. - Added unit tests to validate default vs
fullWidthmax-width/gutter behavior forPageWithTitle.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/javascript/bh-shared-ui/src/components/PageWithTitle.tsx | Stops overriding horizontal padding so Container default gutters handle responsive spacing consistently. |
| packages/javascript/bh-shared-ui/src/components/PageWithTitle.test.tsx | Adds focused tests asserting max-width and gutter behavior for default and fullWidth rendering. |
| cmd/ui/src/views/Administration/Administration.tsx | Disables outer Administration container gutters to avoid double horizontal padding when nested pages use PageWithTitle. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| return ( | ||
| <Box className='flex h-full'> | ||
| <div className='flex h-full'> |
There was a problem hiding this comment.
Did this Box -> div since I know we're trying to inch away from MUI, and this is essentially a noop
Description
PageWithTitleat every viewport width, including whenfullWidthremoves the maximum-width constraint.PageWithTitlepages receive exactly one set of horizontal gutters.fullWidthcontainer behavior with focused unit tests.Box, which only supplied aclassName, with a nativedivas a small step away from unnecessary MUI wrappers.Motivation and Context
Resolves BED-9094
PageWithTitlecurrently appliespx-0 xl:px-6, which removes all horizontal page spacing below thexlbreakpoint. Restoring the Container's default gutters fixes pages such as Download Collectors, My Profile, and API Explorer, but Administration already supplies its own Container gutters. Disabling those outer gutters makesPageWithTitlethe single owner of page spacing and avoids doubled padding on pages such as Data Quality and Manage Users.How Has This Been Tested?
Automated validation:
corepack yarn --cwd bhce workspace bh-shared-ui test PageWithTitle.test.tsx --run— 6 tests passedjust -f bhce/justfile prepare-for-codereviewjust prepare-for-codereviewExtensive manual testing has also been done in both BHE and BHCE.
Regression coverage from BED-9094:
fullWidthpages retain their responsive gutters without anxlmaximum width.Screenshots (optional):
See https://github.com/SpecterOps/bloodhound-enterprise/pull/1652
Types of changes
Checklist:
Summary by CodeRabbit
Summary by CodeRabbit
Bug Fixes / Improvements
PageWithTitlelayout classes to improve consistent spacing while preservingmaxWidthbehavior and gutter handling.maxWidth='xl'.Tests
PageWithTitledefault vs.fullWidthbehavior for gutters andmaxWidth.