Skip to content

Wizard: Tighten row spacing and hide idle password hints (HMS-10793) - #4644

Open
mgold1234 wants to merge 1 commit into
osbuild:mainfrom
mgold1234:fix_spacing
Open

Wizard: Tighten row spacing and hide idle password hints (HMS-10793)#4644
mgold1234 wants to merge 1 commit into
osbuild:mainfrom
mgold1234:fix_spacing

Conversation

@mgold1234

@mgold1234 mgold1234 commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

Reset table row offset in Users and Groups for neater input spacing.
Only show password validation helper text when the input is focused
or has errors.
Before:
Screenshot 2026-07-22 at 18 42 19
After:
Screenshot 2026-07-22 at 18 41 26

JIRA: HMS-10793

@codecov

codecov Bot commented Jul 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.80%. Comparing base (9fecf11) to head (5f2b103).

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4644      +/-   ##
==========================================
- Coverage   77.85%   77.80%   -0.05%     
==========================================
  Files         264      264              
  Lines        7066     7066              
  Branches     2603     2603              
==========================================
- Hits         5501     5498       -3     
- Misses       1471     1474       +3     
  Partials       94       94              
Flag Coverage Δ
playwright 59.81% <ø> (-0.10%) ⬇️
vitest 72.91% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...ageWizard/steps/UserGroups/components/GroupRow.tsx 96.96% <ø> (ø)
...eateImageWizard/steps/Users/components/UserRow.tsx 79.59% <ø> (ø)

... and 3 files with indirect coverage changes


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9fecf11...5f2b103. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mgold1234
mgold1234 force-pushed the fix_spacing branch 5 times, most recently from 8f61cc4 to edffb94 Compare July 22, 2026 14:27
@mgold1234 mgold1234 changed the title Wizard: Remove compact table variant for neater input spacing (HMS-10793) Wizard: Tune compact table padding and condense password helper text (HMS-10793) Jul 22, 2026
@mgold1234
mgold1234 force-pushed the fix_spacing branch 2 times, most recently from 044f02f to 5d291e3 Compare July 22, 2026 15:46
@mgold1234
mgold1234 marked this pull request as ready for review July 22, 2026 15:46
@mgold1234
mgold1234 requested a review from a team as a code owner July 22, 2026 15:46
@mgold1234
mgold1234 requested review from ksiekl, ochosi and regexowl July 22, 2026 15:46

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've left some high level feedback:

  • The inline table padding overrides are duplicated in both UserInfo and GroupInfo; consider extracting these CSS custom properties into a shared style or class to avoid repetition and keep future tuning in one place.
  • In PasswordValidatedInput, hasError treats ruleLength === 'indeterminate' as an error but not ruleCharacters === 'indeterminate; it may be worth aligning the behavior of these two rules or documenting why they differ.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The inline table padding overrides are duplicated in both UserInfo and GroupInfo; consider extracting these CSS custom properties into a shared style or class to avoid repetition and keep future tuning in one place.
- In `PasswordValidatedInput`, `hasError` treats `ruleLength === 'indeterminate'` as an error but not `ruleCharacters === 'indeterminate`; it may be worth aligning the behavior of these two rules or documenting why they differ.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@mgold1234
mgold1234 force-pushed the fix_spacing branch 2 times, most recently from 30cae2d to 6b93726 Compare July 26, 2026 13:06
@mgold1234

Copy link
Copy Markdown
Collaborator Author

/retest

@ksiekl ksiekl left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hiya! I took a look at the PR, and I'm unsure if the compactTableStyle.ts is the way we want to go - generally, we don't mess with the CSS properties if we can avoid it. And if I understand correctly, this new file, together with the resetOffset, just changes the padding of the table - but to be honest, I do not see the difference between before and after. Maybe I'm missing something, if so, feel free to correct me!

And I noticed a problem with the validation, which should definitely be fixed, but could be a follow-up.

Comment thread src/Components/CreateImageWizard/utilities/PasswordValidatedInput.tsx Outdated
@mgold1234 mgold1234 changed the title Wizard: Tune compact table padding and condense password helper text (HMS-10793) Wizard: Tighten row spacing and hide idle password hints (HMS-10793) Aug 10, 2026
@mgold1234

Copy link
Copy Markdown
Collaborator Author

/retest

@regexowl

Copy link
Copy Markdown
Collaborator

I can't see much difference between the changes and the users table that's on stage. Is that intentional? (this is screencast of clicking between changes ran locally and changes on stage)

users-table

But important information for the users step - the UX team presented new mocks last week during the UX/dev sync and we'll be most probably re-revamping the user step (and firewall) to give each user more space. I don't think the mocks are final yet, but complete overhaul of the step is probably in the near future.

There are two options - either we merge this, update the follow up to fix the regression and then wait for the new mocks. Or leave the user step as is and just wait for the new mocks.

@mgold1234

Copy link
Copy Markdown
Collaborator Author

@regexowl up to you, what do you prefer? merge it or wait?

@mgold1234

Copy link
Copy Markdown
Collaborator Author

@regexowl can we merge it?
and update the foloow up to fix the regression ?

@regexowl

Copy link
Copy Markdown
Collaborator

Since the only difference between the tables is the offset, let's maybe just leave the resetOffset. Changes to the validation logic which don't affect rendering can be removed and the tests restored. The follow up seems to be fixing an existing regression, but it's independent on these changes, is that correct?

Add resetOffset to Tr components in UserRow and GroupRow to reduce
spacing around inputs.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants