EDM-3320: Improve alignment in Device table rows#628
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughApply consistent inline and padding styling to several PatternFly Buttons, remove the ChangesButton Styling Updates
CopyButton Refactoring
ApproveDeviceForm Rendering
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
libs/ui-components/src/components/Device/DeviceDetails/DeviceFleet.tsx (2)
25-25: Prefer a CSS class over inline style for consistency.
DeviceFleet.cssis already imported for this component (line 14). MovingpaddingBlock: 0into a class there would keep styling concerns co-located and avoid scattering inline styles; same applies to the duplicated usage inCopyButton.tsx.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/ui-components/src/components/Device/DeviceDetails/DeviceFleet.tsx` at line 25, The inline style setting paddingBlock: 0 in the DeviceFleet JSX should be replaced with a CSS class; add a descriptive class (e.g., .padding-block-0 or .no-padding-block) to DeviceFleet.css, apply that class to the same element in the DeviceFleet component (replace the style={{ paddingBlock: 0 }} usage), and update the duplicated instance in the CopyButton component to use the same CSS class so both components share the styling from DeviceFleet.css.
22-28: Consider applying the same padding tweak toMultipleDeviceOwners.The sibling inline
Buttonat lines 63–72 uses the sameisInline+variant="plain"pattern but doesn't getpaddingBlock: 0. If vertical alignment is the goal, both icon buttons likely need the same treatment to avoid subtle row-height drift when the multi-owner warning is present. If this was intentional (only the fleetless case needed the fix in practice), please ignore.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/ui-components/src/components/Device/DeviceDetails/DeviceFleet.tsx` around lines 22 - 28, The inline plain icon Button used inside the MultipleDeviceOwners rendering should receive the same vertical padding tweak as the other icon Button to keep row height consistent; locate the Button instance inside the MultipleDeviceOwners block (the sibling to the first Button with isInline and variant="plain") and add style={{ paddingBlock: 0 }} (or the project's equivalent spacing prop) to that Button so both icon buttons share identical vertical alignment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@libs/ui-components/src/components/Device/DeviceDetails/DeviceFleet.tsx`:
- Line 25: The inline style setting paddingBlock: 0 in the DeviceFleet JSX
should be replaced with a CSS class; add a descriptive class (e.g.,
.padding-block-0 or .no-padding-block) to DeviceFleet.css, apply that class to
the same element in the DeviceFleet component (replace the style={{
paddingBlock: 0 }} usage), and update the duplicated instance in the CopyButton
component to use the same CSS class so both components share the styling from
DeviceFleet.css.
- Around line 22-28: The inline plain icon Button used inside the
MultipleDeviceOwners rendering should receive the same vertical padding tweak as
the other icon Button to keep row height consistent; locate the Button instance
inside the MultipleDeviceOwners block (the sibling to the first Button with
isInline and variant="plain") and add style={{ paddingBlock: 0 }} (or the
project's equivalent spacing prop) to that Button so both icon buttons share
identical vertical alignment.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4ea1238f-7a8c-4d54-8ec9-87d76fce996b
📒 Files selected for processing (4)
libs/ui-components/src/components/Device/DeviceDetails/DeviceFleet.tsxlibs/ui-components/src/components/EnrollmentRequest/EnrollmentRequestTableRow.tsxlibs/ui-components/src/components/common/CopyButton.tsxlibs/ui-components/src/components/modals/ApproveDeviceModal/ApproveDeviceForm.tsx
celdrake
force-pushed
the
EDM-3320-better-device-table-alignment
branch
from
752c3a7 to
16bc404
Compare
celdrake
force-pushed
the
EDM-3320-better-device-table-alignment
branch
from
16bc404 to
a7c9e0e
Compare
2 participants
Simplified display to make all columns in EnrolledDevicesTable vertically aligned.
See screenshot with added guidelines to validate alignment is correct.
Summary by CodeRabbit
Style
Refactor