Skip to content

fix: Fixed inconsistent alignment of titles and headings in status panel #23160

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sivasath16
Copy link

Fixes #23022

Checklist:

  • Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
  • The title of the PR states what changed and the related issues number (used for the release note).
  • The title of the PR conforms to the Toolchain Guide
  • I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue.
  • I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
  • Does this PR require documentation updates?
  • I've updated documentation as required by this PR.
  • I have signed off all my commits as required by DCO
  • I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
  • My build is green (troubleshooting builds).
  • My new feature complies with the feature status guidelines.
  • I have added a brief description of why this PR is necessary and/or what this PR solves.
  • Optional. My organization is added to USERS.md.
  • Optional. For bug fixes, I've indicated what older releases this fix should be cherry-picked into (this may or may not happen depending on risk/complexity).

@sivasath16 sivasath16 requested a review from a team as a code owner May 26, 2025 23:38
Copy link

bunnyshell bot commented May 26, 2025

🔴 Preview Environment stopped on Bunnyshell

See: Environment Details | Pipeline Logs

Available commands (reply to this comment):

  • 🔵 /bns:start to start the environment
  • 🚀 /bns:deploy to redeploy the environment
  • /bns:delete to remove the environment

@sivasath16
Copy link
Author

@reggie-k, I've fixed the UI bug. Can you take a look at it?

@surajyadav1108
Copy link
Contributor

It would be better if you could please provide a before and after of the UI changes.

Copy link
Member

@nitishfy nitishfy left a comment

Choose a reason for hiding this comment

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

I tested it locally.

Before

Screenshot 2025-05-27 at 3 04 29 PM

After

Screenshot 2025-05-27 at 2 58 42 PM

I don't see much of a difference here.

@crenshaw-dev
Copy link
Member

The easiest way to test the worst of the alignment issues (the conditions panel) is to intentionally add a typo to the source.repoURL field. I tested with that situation, looks like this PR doesn't yet address that alignment issue.
image

@@ -47,7 +47,7 @@ const sectionLabel = (info: SectionInfo) => (

const sectionHeader = (info: SectionInfo, onClick?: () => any) => {
return (
<div style={{display: 'flex', alignItems: 'center', marginBottom: '0.5em'}}>
<div style={{display: 'flex', alignItems: 'center'}}>
Copy link
Member

Choose a reason for hiding this comment

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

Some status headers could support buttons to enable more actions. To maintain visual alignment across both cases, I recommend using a fixed row height of 20px and avoiding fractional line heights.

Suggested change
<div style={{display: 'flex', alignItems: 'center'}}>
<div style={{display: 'flex', alignItems: 'center', lineHeight: '20px'}}>

I noticed the remaining sectionLabel usages at:

<div className='application-status-panel__item' style={{position: 'relative'}}>
{sectionLabel({
title: 'SYNC WINDOWS',
helpContent:
'The aggregate state of sync windows for this app. ' +
'Red: no syncs allowed. ' +
'Yellow: manual syncs allowed. ' +
'Green: all syncs allowed'
})}
<div className='application-status-panel__item-value' style={{margin: 'auto 0'}}>
<ApplicationSyncWindowStatusIcon project={application.spec.project} state={data} />
</div>

<div style={{lineHeight: '19.5px', marginBottom: '0.3em'}}>
{sectionLabel({
title: 'SOURCE HYDRATOR',
helpContent: 'The source hydrator reads manifests from git, hydrates (renders) them, and pushes them to a different location in git.'
})}
</div>

Also, there's an unusual marginTop defined in application-status-panel.scss, which I'd suggest removing to simplify the layout.

@@ -156,7 +156,8 @@ export const ApplicationStatusPanel = ({application, showDiff, showOperation, sh
return (
<div className='application-status-panel row'>
<div className='application-status-panel__item'>
<div style={{lineHeight: '19.5px', marginBottom: '0.3em'}}>{sectionLabel({title: 'APP HEALTH', helpContent: 'The health status of your app'})}</div>
{/* <div style={{lineHeight: '19.5px', marginBottom: '0.3em'}}>{sectionLabel({title: 'APP HEALTH', helpContent: 'The health status of your app'})}</div> */}
{sectionHeader({title: 'APP HEALTH', helpContent: 'The health status of your app'})}
<div className='application-status-panel__item-value'>
Copy link
Member

Choose a reason for hiding this comment

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

Additionally, for the application-status-panel__item-value className, I recommend constraining its layout using a min-height rather than fixed heights. Since this element is based on a flex layout, using a consistent height will ensure vertical alignment across items. From experiments, a min-height: 40px works well for most cases, except for appConditions, which includes multiple status labels and may require special handling.

To fine-tune the alignment, I suggest adding margin-top: 1px; around this line (L50)

, and adding vertical-align: middle; at this line (L190) to ensure the status icon in the conditions section is vertically aligned with the text.

Also, I recommend removing the line margin: auto;

, as it causes vertical misalignment due to its effect on the element's positioning within flex containers.

With these adjustments, we should be able to achieve a more consistent and visually aligned layout.

image

For comparison, here's a screenshot of the previous layout before these adjustments:

image

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.

Inconsistent alignment in app panel
5 participants