chore: Hodge Podge to address various CSS inconsistencies - BED-7297 #2386
Open
KillahDillah wants to merge 18 commits intomainfrom
Open
chore: Hodge Podge to address various CSS inconsistencies - BED-7297 #2386KillahDillah wants to merge 18 commits intomainfrom
KillahDillah wants to merge 18 commits intomainfrom
Conversation
KillahDillah
added
the
user interface
Contributor
|
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:
📝 WalkthroughWalkthroughThis PR applies UI/layout-only changes across multiple components: Tailwind class tweaks (rounded → rounded-lg, border-y → border-b), padding/margin adjustments, height/width constraint updates, and minor structural rearrangements. No logic, state, or public API signatures were changed. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
KillahDillah
force-pushed
the
BED-7297-styling-bugs
branch
2 times, most recently
from
cb132ab to
2883ed5
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/javascript/bh-shared-ui/src/views/PrivilegeZones/Details/ObjectsAccordion.tsx (1)
96-103:⚠️ Potential issue | 🟡 Minor
LoadingRowborder style is now inconsistent withRow.
LoadingRow(Line 100) still usesborder-y, while the dataRow(Line 130) was updated toborder-b. This causes a visible border difference between the skeleton loading state and the populated state — each loading row has both a top and bottom border, but each data row only has a bottom border.🔧 Proposed fix
- className='border-y border-neutral-3 relative w-full p-2'> + className='border-b border-neutral-3 relative w-full p-2'>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/javascript/bh-shared-ui/src/views/PrivilegeZones/Details/ObjectsAccordion.tsx` around lines 96 - 103, The loading skeleton's container (LoadingRow) uses 'border-y' which creates both top and bottom borders and mismatches the actual data row (Row) that uses 'border-b'; update the LoadingRow component's className to use 'border-b' instead of 'border-y' so the skeleton row visually matches the data Row (look for the LoadingRow arrow function and the Row component in ObjectsAccordion.tsx and change the className accordingly).
🧹 Nitpick comments (9)
packages/javascript/bh-shared-ui/src/views/PrivilegeZones/History/HistoryNote.tsx (1)
35-35: Consider applyingrounded-lgto the content Card for consistency.Every other Card in this PR's touch list (e.g.,
BasicInfo.tsx,SeedSelection.tsx,ObjectCountPanel.tsx) receivesrounded-lg. The inner content Card here is the only one that doesn't, which may produce a visual inconsistency (square-cornered note body beneath a rounded-lg header card).✨ Proposed fix
- <Card className='p-4'> + <Card className='p-4 rounded-lg'>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/javascript/bh-shared-ui/src/views/PrivilegeZones/History/HistoryNote.tsx` at line 35, In HistoryNote.tsx update the inner content Card (the JSX element using Card with className='p-4') to include rounded-lg in its className so it matches the other Cards (e.g., BasicInfo, SeedSelection, ObjectCountPanel); modify the Card's className string to add 'rounded-lg' alongside 'p-4' to restore consistent rounded corners for the note body.packages/javascript/bh-shared-ui/src/views/PrivilegeZones/Details/ObjectCountPanel.tsx (1)
27-81: Consider extracting the shared CardclassNameinto a constant.The loading and error Card classNames are identical, and the success branch differs only by
overflow-y-auto. Extracting these reduces duplication and makes future styling updates a single-line change.♻️ Proposed refactor
+const baseCardClassName = 'flex flex-col px-6 py-6 select-none rounded-lg'; + const ObjectCountPanel: FC = () => { const objectsCountQuery = useObjectCounts(); if (objectsCountQuery.isLoading) { return ( <Card tabIndex={0} - className='flex flex-col px-6 py-6 select-none rounded-lg' + className={baseCardClassName} data-testid='privilege-zones_object-counts'> ... } else if (objectsCountQuery.isError) { return ( <Card tabIndex={0} - className='flex flex-col px-6 py-6 select-none rounded-lg' + className={baseCardClassName} data-testid='privilege-zones_object-counts'> ... } else if (objectsCountQuery.isSuccess && objectsCountQuery.data) { return ( <Card tabIndex={0} - className='flex flex-col px-6 py-6 select-none overflow-y-auto rounded-lg' + className={`${baseCardClassName} overflow-y-auto`} data-testid='privilege-zones_object-counts'>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/javascript/bh-shared-ui/src/views/PrivilegeZones/Details/ObjectCountPanel.tsx` around lines 27 - 81, The Card className is duplicated across branches (loading, error, success) with only the success branch adding overflow-y-auto; extract the shared string into a constant (e.g., baseCardClass) and replace each Card's className with baseCardClass, using `${baseCardClass} overflow-y-auto` (or equivalent) in the objectsCountQuery.isSuccess branch; update the Card usages in this file (the loading branch, the objectsCountQuery.isError branch, and the objectsCountQuery.isSuccess branch) to reference that constant so future styling changes are made in one place.packages/javascript/bh-shared-ui/src/views/PrivilegeZones/Save/RuleForm/SeedSelection.tsx (1)
115-115:pl-4is redundant alongsidepx-4.
px-4already sets bothpadding-leftandpadding-rightto1rem, sopl-4is a no-op here and can be removed.♻️ Proposed cleanup
- <Card className='mb-5 pl-4 px-4 py-2 rounded-lg'> + <Card className='mb-5 px-4 py-2 rounded-lg'>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/javascript/bh-shared-ui/src/views/PrivilegeZones/Save/RuleForm/SeedSelection.tsx` at line 115, The Card element in SeedSelection.tsx uses both "px-4" and "pl-4" in its className, making "pl-4" redundant; remove "pl-4" from the className on the Card (the JSX element with className='mb-5 pl-4 px-4 py-2 rounded-lg') so it becomes 'mb-5 px-4 py-2 rounded-lg'.cmd/ui/src/views/Explore/ExploreSearch/ExploreSearch.tsx (1)
172-216: Verify the intentional mix ofroundedvsrounded-lgwithin this component.The header div (line 178) was updated to
rounded-lg, but the outer container (line 174) and the content panel (line 213) still userounded. If full visual consistency is the goal, these two spots may have been missed.♻️ Potential follow-up if a uniform corner radius is desired
- className={cn('h-full min-h-0 w-[410px] flex gap-4 flex-col rounded', { + className={cn('h-full min-h-0 w-[410px] flex gap-4 flex-col rounded-lg', {- className={cn('hidden min-h-0 rounded pointer-events-auto', { + className={cn('hidden min-h-0 rounded-lg pointer-events-auto', {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/ui/src/views/Explore/ExploreSearch/ExploreSearch.tsx` around lines 172 - 216, The component mixes Tailwind corner utilities: the outer wrapper (data-testid='explore_search-container') and content panel (the div with className computed via cn starting with 'hidden min-h-0 rounded pointer-events-auto') use "rounded" while the header (data-testid='explore_search-container_header') uses "rounded-lg"; pick the intended radius and make them consistent by changing the className on either the header to "rounded" or the outer/container divs to "rounded-lg" so all three (outer wrapper, header, content panel) match; update the className strings on ExploreSearch's JSX accordingly.packages/javascript/bh-shared-ui/src/components/VirtualizedNodeList.tsx (1)
87-87:border-y-neutral-light-5color class doesn't match theborder-bwidth class.After changing
border-y→border-b, the color utility was not updated correspondingly.border-y-neutral-light-5sets the color for both top and bottom borders; with onlyborder-bin play,border-b-neutral-light-5is the precise match. Functionally the bottom border renders correctly sinceborder-y-{color}also setsborder-bottom-color, but the semantic mismatch is misleading.♻️ Proposed fix
- 'bg-neutral-light-2 dark:bg-neutral-dark-2 flex items-center pl-2 border-b border-y-neutral-light-5', + 'bg-neutral-light-2 dark:bg-neutral-dark-2 flex items-center pl-2 border-b border-b-neutral-light-5',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/javascript/bh-shared-ui/src/components/VirtualizedNodeList.tsx` at line 87, The CSS utility class for the border color is semantically mismatched: in VirtualizedNodeList.tsx update the class string that currently contains 'border-y-neutral-light-5' to use 'border-b-neutral-light-5' so the color utility matches the explicit bottom-border width class (look for the class list containing 'bg-neutral-light-2 dark:bg-neutral-dark-2 flex items-center pl-2 border-b ...' and replace the color token accordingly).packages/javascript/bh-shared-ui/src/views/PrivilegeZones/Details/Details.tsx (1)
60-60: Consider documenting the magic10remoffset.The
10reminh-[calc(100%-10rem)]presumably accounts for fixed chrome above this component (top nav, breadcrumbs, etc.), but there's no comment or named token making that assumption explicit. If the surrounding layout changes, this silently mis-sizes the panel.🛠️ Suggestion
- <div className='h-[calc(100%-10rem)]'> + {/* 10rem = combined height of the app topbar + page breadcrumbs */} + <div className='h-[calc(100%-10rem)]'>Or extract to a Tailwind CSS variable / utility class if the same offset is reused elsewhere.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/javascript/bh-shared-ui/src/views/PrivilegeZones/Details/Details.tsx` at line 60, The hard-coded height calc using the Tailwind class h-[calc(100%-10rem)] in Details.tsx is a magic value; replace it with a documented and reusable approach by either adding an inline comment explaining that 10rem accounts for the fixed top chrome (nav/breadcrumbs) or extracting the offset into a named constant/utility (e.g., a Tailwind variable/utility class or a JS/TS constant like PANEL_TOP_OFFSET) and using that in the height calculation so the intent is clear and changes to the layout only require updating one place.packages/javascript/bh-shared-ui/src/views/PrivilegeZones/Details/DynamicDetails.tsx (1)
131-131:max-h-fullremoved fromRuleDetails— intentional asymmetry withTagDetails?
TagDetails(line 80) still carriesmax-h-full, whileRuleDetailsno longer does. The parent column inDetails.tsx(line 79) ownsoverflow-y-auto, so there's no functional overflow risk. If this is intentional (e.g., to fix the rule-creation overflow bug mentioned in the PR), a brief inline comment would help future readers understand the divergence.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/javascript/bh-shared-ui/src/views/PrivilegeZones/Details/DynamicDetails.tsx` at line 131, RuleDetails lost the max-h-full utility while TagDetails still has it, creating an unexplained asymmetry; either restore max-h-full on the RuleDetails container or, if the removal was intentional to address the rule-creation overflow bug, add an inline comment near the RuleDetails JSX (and reference Details.tsx parent column with overflow-y-auto) explaining why RuleDetails omits max-h-full so future readers understand the divergence from TagDetails.packages/javascript/bh-shared-ui/src/views/PrivilegeZones/History/HistoryContent.tsx (2)
94-95: Fragile height pairing:h-[75dvh]outer andh-[65dvh]inner are independently hardcoded.The 10dvh gap is expected to absorb the
CardHeaderheight plus any padding. This assumption breaks if header or padding sizes change, or on very short viewports where 10dvh could be smaller than the actual header height (e.g., at 600px viewport height, 10dvh ≈ 60px which may be insufficient).A more resilient approach is to make the
Carda flex column and let the scroll area grow to fill remaining space:♻️ Proposed refactor to use flex layout instead of paired hardcoded heights
- <div data-testid='history-wrapper' className='flex gap-6 mt-4 h-[75dvh]'> - <Card className='rounded-lg flex-1 min-w-0'> + <div data-testid='history-wrapper' className='flex gap-6 mt-4 h-[75dvh]'> + <Card className='rounded-lg flex-1 min-w-0 flex flex-col overflow-hidden'> <CardHeader className='flex-row ml-3 justify-between items-center'> ... </CardHeader> <div onScroll={...} ref={scrollRef} tabIndex={0} - className='overflow-y-auto h-[65dvh]'> + className='overflow-y-auto flex-1'>Also applies to: 104-109
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/javascript/bh-shared-ui/src/views/PrivilegeZones/History/HistoryContent.tsx` around lines 94 - 95, The layout uses fragile hardcoded heights (h-[75dvh] on the outer div and h-[65dvh] on an inner element) which can break if CardHeader or paddings change; update HistoryContent.tsx to make the Card a flex column and replace fixed inner height with a flex-grow scrollable container so the header size is intrinsic and the content area fills remaining space (adjust the element that currently uses h-[65dvh] to use flex-1/overflow-auto and set Card to flex flex-col); target the outer wrapper with data-testid='history-wrapper', the Card component instance, and the scroll area inside the Card to implement these changes.
121-123: Fixedw-[400px] min-w-[400px]on theHistoryNotecolumn is not responsive.On narrower viewports or when the browser window is resized below ~450px effective width, the fixed 400px sidebar and the unsized main card will overflow or collapse the layout with no wrapping or responsive fallback.
Consider whether a minimum breakpoint guard or a
max-wwithshrinkis warranted, or document that this view has a known minimum supported viewport width.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/javascript/bh-shared-ui/src/views/PrivilegeZones/History/HistoryContent.tsx` around lines 121 - 123, The fixed 400px sidebar width on the div wrapping HistoryNote causes layout overflow on narrow viewports; update the wrapper (the div that currently has 'w-[400px] min-w-[400px] overflow-y-auto') to use responsive and shrinkable sizing so it can collapse on small screens (for example use a full-width fallback like w-full with a sm:w-[400px] or max-w-[400px], and ensure min-w-0 and flex-shrink-0 are applied so the HistoryNote column can shrink instead of forcing overflow). Locate the wrapper around the HistoryNote component in HistoryContent.tsx and replace the fixed width classes with responsive/min/max/shrink classes to prevent overflow on narrow viewports.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/javascript/bh-shared-ui/src/views/PrivilegeZones/History/HistoryContent.tsx`:
- Around line 94-95: The Card inside the HistoryContent component (the element
under the div with data-testid='history-wrapper') lacks a flex rule so it
doesn't expand in the flex row; update the Card's className to include "flex-1
min-w-0" (i.e., change Card className to 'flex-1 min-w-0 rounded-lg' or append
those utilities) so it fills remaining horizontal space—apply the same change to
the other Card instance referenced in this file (the second Card near the
History log section).
---
Outside diff comments:
In
`@packages/javascript/bh-shared-ui/src/views/PrivilegeZones/Details/ObjectsAccordion.tsx`:
- Around line 96-103: The loading skeleton's container (LoadingRow) uses
'border-y' which creates both top and bottom borders and mismatches the actual
data row (Row) that uses 'border-b'; update the LoadingRow component's className
to use 'border-b' instead of 'border-y' so the skeleton row visually matches the
data Row (look for the LoadingRow arrow function and the Row component in
ObjectsAccordion.tsx and change the className accordingly).
---
Nitpick comments:
In `@cmd/ui/src/views/Explore/ExploreSearch/ExploreSearch.tsx`:
- Around line 172-216: The component mixes Tailwind corner utilities: the outer
wrapper (data-testid='explore_search-container') and content panel (the div with
className computed via cn starting with 'hidden min-h-0 rounded
pointer-events-auto') use "rounded" while the header
(data-testid='explore_search-container_header') uses "rounded-lg"; pick the
intended radius and make them consistent by changing the className on either the
header to "rounded" or the outer/container divs to "rounded-lg" so all three
(outer wrapper, header, content panel) match; update the className strings on
ExploreSearch's JSX accordingly.
In `@packages/javascript/bh-shared-ui/src/components/VirtualizedNodeList.tsx`:
- Line 87: The CSS utility class for the border color is semantically
mismatched: in VirtualizedNodeList.tsx update the class string that currently
contains 'border-y-neutral-light-5' to use 'border-b-neutral-light-5' so the
color utility matches the explicit bottom-border width class (look for the class
list containing 'bg-neutral-light-2 dark:bg-neutral-dark-2 flex items-center
pl-2 border-b ...' and replace the color token accordingly).
In
`@packages/javascript/bh-shared-ui/src/views/PrivilegeZones/Details/Details.tsx`:
- Line 60: The hard-coded height calc using the Tailwind class
h-[calc(100%-10rem)] in Details.tsx is a magic value; replace it with a
documented and reusable approach by either adding an inline comment explaining
that 10rem accounts for the fixed top chrome (nav/breadcrumbs) or extracting the
offset into a named constant/utility (e.g., a Tailwind variable/utility class or
a JS/TS constant like PANEL_TOP_OFFSET) and using that in the height calculation
so the intent is clear and changes to the layout only require updating one
place.
In
`@packages/javascript/bh-shared-ui/src/views/PrivilegeZones/Details/DynamicDetails.tsx`:
- Line 131: RuleDetails lost the max-h-full utility while TagDetails still has
it, creating an unexplained asymmetry; either restore max-h-full on the
RuleDetails container or, if the removal was intentional to address the
rule-creation overflow bug, add an inline comment near the RuleDetails JSX (and
reference Details.tsx parent column with overflow-y-auto) explaining why
RuleDetails omits max-h-full so future readers understand the divergence from
TagDetails.
In
`@packages/javascript/bh-shared-ui/src/views/PrivilegeZones/Details/ObjectCountPanel.tsx`:
- Around line 27-81: The Card className is duplicated across branches (loading,
error, success) with only the success branch adding overflow-y-auto; extract the
shared string into a constant (e.g., baseCardClass) and replace each Card's
className with baseCardClass, using `${baseCardClass} overflow-y-auto` (or
equivalent) in the objectsCountQuery.isSuccess branch; update the Card usages in
this file (the loading branch, the objectsCountQuery.isError branch, and the
objectsCountQuery.isSuccess branch) to reference that constant so future styling
changes are made in one place.
In
`@packages/javascript/bh-shared-ui/src/views/PrivilegeZones/History/HistoryContent.tsx`:
- Around line 94-95: The layout uses fragile hardcoded heights (h-[75dvh] on the
outer div and h-[65dvh] on an inner element) which can break if CardHeader or
paddings change; update HistoryContent.tsx to make the Card a flex column and
replace fixed inner height with a flex-grow scrollable container so the header
size is intrinsic and the content area fills remaining space (adjust the element
that currently uses h-[65dvh] to use flex-1/overflow-auto and set Card to flex
flex-col); target the outer wrapper with data-testid='history-wrapper', the Card
component instance, and the scroll area inside the Card to implement these
changes.
- Around line 121-123: The fixed 400px sidebar width on the div wrapping
HistoryNote causes layout overflow on narrow viewports; update the wrapper (the
div that currently has 'w-[400px] min-w-[400px] overflow-y-auto') to use
responsive and shrinkable sizing so it can collapse on small screens (for
example use a full-width fallback like w-full with a sm:w-[400px] or
max-w-[400px], and ensure min-w-0 and flex-shrink-0 are applied so the
HistoryNote column can shrink instead of forcing overflow). Locate the wrapper
around the HistoryNote component in HistoryContent.tsx and replace the fixed
width classes with responsive/min/max/shrink classes to prevent overflow on
narrow viewports.
In
`@packages/javascript/bh-shared-ui/src/views/PrivilegeZones/History/HistoryNote.tsx`:
- Line 35: In HistoryNote.tsx update the inner content Card (the JSX element
using Card with className='p-4') to include rounded-lg in its className so it
matches the other Cards (e.g., BasicInfo, SeedSelection, ObjectCountPanel);
modify the Card's className string to add 'rounded-lg' alongside 'p-4' to
restore consistent rounded corners for the note body.
In
`@packages/javascript/bh-shared-ui/src/views/PrivilegeZones/Save/RuleForm/SeedSelection.tsx`:
- Line 115: The Card element in SeedSelection.tsx uses both "px-4" and "pl-4" in
its className, making "pl-4" redundant; remove "pl-4" from the className on the
Card (the JSX element with className='mb-5 pl-4 px-4 py-2 rounded-lg') so it
becomes 'mb-5 px-4 py-2 rounded-lg'.
packages/javascript/bh-shared-ui/src/views/PrivilegeZones/History/HistoryContent.tsx
Outdated
Show resolved
Hide resolved
KillahDillah
force-pushed
the
BED-7297-styling-bugs
branch
from
2883ed5 to
230ccdf
Compare
KillahDillah
force-pushed
the
BED-7297-styling-bugs
branch
from
230ccdf to
0a8253f
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/javascript/bh-shared-ui/src/components/VirtualizedNodeList.tsx (1)
87-87:border-y-neutral-light-5color class is semantically stale after switching toborder-b
border-y-neutral-light-5sets the color for both the top and bottom borders, but now thatborder-bis the only active width class the top-border color is silently ignored. It's harmless, but updating the color utility toborder-b-neutral-light-5makes the intent self-documenting.♻️ Align the border-color class with border-b
-'bg-neutral-light-2 dark:bg-neutral-dark-2 flex items-center pl-2 border-b border-y-neutral-light-5', +'bg-neutral-light-2 dark:bg-neutral-dark-2 flex items-center pl-2 border-b border-b-neutral-light-5',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/javascript/bh-shared-ui/src/components/VirtualizedNodeList.tsx` at line 87, The CSS utility for the border color is still using border-y-neutral-light-5 while the markup only applies a bottom border via border-b; in VirtualizedNodeList (the JSX element with class string 'bg-neutral-light-2 dark:bg-neutral-dark-2 flex items-center pl-2 border-b border-y-neutral-light-5') replace border-y-neutral-light-5 with border-b-neutral-light-5 so the border color utility matches the active border width utility and documents the intent.packages/javascript/bh-shared-ui/src/views/PrivilegeZones/Save/RuleForm/SeedSelection.tsx (1)
115-115: Nit:pl-4is redundant whenpx-4is present.
px-4already sets bothpadding-leftandpadding-rightto1rem, making the explicitpl-4unnecessary. This may have been pre-existing, but since the line was touched, it's a good opportunity to clean it up.Suggested fix
- <Card className='mb-5 pl-4 px-4 py-2 rounded-lg'> + <Card className='mb-5 px-4 py-2 rounded-lg'>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/javascript/bh-shared-ui/src/views/PrivilegeZones/Save/RuleForm/SeedSelection.tsx` at line 115, In SeedSelection.tsx, the Card component's className includes a redundant "pl-4" alongside "px-4"; remove "pl-4" from the className on the Card (the JSX element using Card in the RuleForm) so padding-left isn't duplicated—leave "px-4 py-2 mb-5 rounded-lg" (and any other existing classes) intact.packages/javascript/bh-shared-ui/src/views/PrivilegeZones/Details/Details.tsx (1)
60-65: Fragile magic number inh-[calc(100%-10rem)]The
10remoffset is implicitly assumed to equal the combined height of<PageDescription>+ themt-6 w-2/3InfoHeader block +my-4margins from the flex row (line 65). There is no comment documenting this, so any future height change to either of those sibling elements will silently break the layout.Additionally, the inner flex row (line 65) carries
h-full, which tries to occupy the entireh-[calc(100%-10rem)]height. Because<PageDescription>and the InfoHeader are also inside this container (not positioned out-of-flow), the total block height can exceed the outer constraint. Consider addingoverflow-hiddento the outer wrapper or, ideally, replacing this with a proper flex column layout that letsPageDescription/InfoHeadershrink-wrap and the main content area flex-grow:♻️ Suggested structural alternative
-<div className='h-[calc(100%-10rem)]'> +<div className='flex flex-col h-full overflow-hidden'> <PageDescription /> <div className='mt-6 w-2/3'> <InfoHeader /> </div> - <div className='flex gap-6 my-4 h-full'> + <div className='flex gap-6 my-4 flex-1 min-h-0'>This makes the outer wrapper a flex column, lets the static elements size themselves naturally, and gives the main content area the remaining space via
flex-1 min-h-0(themin-h-0is necessary to allow flex children to overflow-scroll correctly).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/javascript/bh-shared-ui/src/views/PrivilegeZones/Details/Details.tsx` around lines 60 - 65, The outer wrapper using the magic height h-[calc(100%-10rem)] is brittle — replace the current static-height approach by making the container a column flex (e.g., add flex flex-col and overflow-hidden), let PageDescription and InfoHeader remain natural flow (remove the hard-coded height dependency), and convert the main content row (the div with class 'flex gap-6 my-4 h-full') into a flex-grow area by giving it flex-1 and min-h-0 (and remove h-full) so it correctly takes remaining space and allows internal scrolling; update class names on the outer wrapper, the InfoHeader container (mt-6 w-2/3) if needed, and the inner flex row to implement this layout change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/javascript/bh-shared-ui/src/components/EntityInfo/EntityInfoPanel.tsx`:
- Line 68: In the EntityInfoPanel component fix the class name typo in the
header wrapper div: replace the incorrect "shadouw-outer-1" with
"shadow-outer-1" in the className string so Tailwind applies the intended
shadow; locate the div in EntityInfoPanel.tsx that currently contains
'bg-neutral-2 pointer-events-auto rounded-lg shadouw-outer-1' and update that
token to 'shadow-outer-1'.
---
Nitpick comments:
In `@packages/javascript/bh-shared-ui/src/components/VirtualizedNodeList.tsx`:
- Line 87: The CSS utility for the border color is still using
border-y-neutral-light-5 while the markup only applies a bottom border via
border-b; in VirtualizedNodeList (the JSX element with class string
'bg-neutral-light-2 dark:bg-neutral-dark-2 flex items-center pl-2 border-b
border-y-neutral-light-5') replace border-y-neutral-light-5 with
border-b-neutral-light-5 so the border color utility matches the active border
width utility and documents the intent.
In
`@packages/javascript/bh-shared-ui/src/views/PrivilegeZones/Details/Details.tsx`:
- Around line 60-65: The outer wrapper using the magic height
h-[calc(100%-10rem)] is brittle — replace the current static-height approach by
making the container a column flex (e.g., add flex flex-col and
overflow-hidden), let PageDescription and InfoHeader remain natural flow (remove
the hard-coded height dependency), and convert the main content row (the div
with class 'flex gap-6 my-4 h-full') into a flex-grow area by giving it flex-1
and min-h-0 (and remove h-full) so it correctly takes remaining space and allows
internal scrolling; update class names on the outer wrapper, the InfoHeader
container (mt-6 w-2/3) if needed, and the inner flex row to implement this
layout change.
In
`@packages/javascript/bh-shared-ui/src/views/PrivilegeZones/Save/RuleForm/SeedSelection.tsx`:
- Line 115: In SeedSelection.tsx, the Card component's className includes a
redundant "pl-4" alongside "px-4"; remove "pl-4" from the className on the Card
(the JSX element using Card in the RuleForm) so padding-left isn't
duplicated—leave "px-4 py-2 mb-5 rounded-lg" (and any other existing classes)
intact.
packages/javascript/bh-shared-ui/src/components/EntityInfo/EntityInfoPanel.tsx
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/javascript/bh-shared-ui/src/components/EntityInfo/EntityInfoPanel.tsx (1)
68-68: Typo fix confirmed —shadow-outer-1is now correctly spelled.The previously flagged
shadouw-outer-1misspelling has been resolved, restoring the intended shadow to the header wrapper.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/javascript/bh-shared-ui/src/components/EntityInfo/EntityInfoPanel.tsx` at line 68, The class name typo has been fixed in the EntityInfoPanel component but please verify and replace any remaining occurrences of the misspelling "shadouw-outer-1" with the correct "shadow-outer-1" inside the EntityInfoPanel JSX (the div with className containing 'bg-neutral-2 pointer-events-auto rounded-lg shadow-outer-1') and run a quick build/test to ensure the intended shadow style is applied everywhere.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@packages/javascript/bh-shared-ui/src/components/EntityInfo/EntityInfoPanel.tsx`:
- Line 68: The class name typo has been fixed in the EntityInfoPanel component
but please verify and replace any remaining occurrences of the misspelling
"shadouw-outer-1" with the correct "shadow-outer-1" inside the EntityInfoPanel
JSX (the div with className containing 'bg-neutral-2 pointer-events-auto
rounded-lg shadow-outer-1') and run a quick build/test to ensure the intended
shadow style is applied everywhere.
ℹ️ Review info
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/javascript/bh-shared-ui/src/components/EntityInfo/EntityInfoPanel.tsxpackages/javascript/bh-shared-ui/src/views/PrivilegeZones/Details/DynamicDetails.tsxpackages/javascript/bh-shared-ui/src/views/PrivilegeZones/Details/ObjectCountPanel.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/javascript/bh-shared-ui/src/views/PrivilegeZones/Details/DynamicDetails.tsx
KillahDillah
commented
Feb 25, 2026
|
|
||
| return ( | ||
| <div className='h-full max-h-[75vh]'> | ||
| <div className='h-[calc(100%-10rem)]'> |
Contributor
Author
There was a problem hiding this comment.
Allows table to grow as a user zooms in and out
KillahDillah
commented
Feb 25, 2026
| <div className='flex items-center' title={name}> | ||
| {glyph && <ZoneIcon zone={tagData} persistGlyph size={20} />} | ||
| <span className='text-xl font-bold truncate'>{name}</span> | ||
| <span className='text-xl font-bold text-wrap'>{name}</span> |
Contributor
Author
There was a problem hiding this comment.
changed this to wrap instead of truncate due to the 'basis' classing, allowing it to grow as the window grows / accommodate for larger screens. If a Zone title is long, instead of being truncated, the entire title will show.
KillahDillah
commented
Feb 25, 2026
Contributor
Author
KillahDillah
left a comment
KillahDillah
left a comment
There was a problem hiding this comment.
Comments left to indicate reasoning of why classes were updated
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
1 participant
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.
Description
This ticket is addressing various CSS issues, mostly within the PZM space.
Motivation and Context
Resolves: BED-7297
For consistency and to fix bugs
How Has This Been Tested?
Resizing the window, zooming in and out, resizing columns on Certs table, expanding and collapsing sections throughout PZM.
Screenshots (optional):
Notice the rounded corners and shadow displaying on EntityInfoPanel
Types of changes
Checklist:
Summary by CodeRabbit