Skip to content

Conversation

@BrilliantDeviation7
Copy link
Contributor

No description provided.

@NoRePercussions
Copy link
Contributor

Screenshot 2026-01-31 at 01 12 09 image

(NB: the wide blocks on mobile aren't a regression, and IME nice)

Copy link
Contributor

@NoRePercussions NoRePercussions left a comment

Choose a reason for hiding this comment

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

Such an improvement! Approved modulo Tomás approval and a nit.

I'd like a clearer commit message. I know you're changing comment styling from the diff, but I can't infer the context I need to write a good review. Here's roughly what I'd have written:

fix: move comment section up to increase visibility

Resolves https://cmuabtech.slack.com/archives/CKLMNB5PX/p1766279256513539.

The comment section on event pages is below other infoboxes,
and falls below the fold when there are many notes/invoices/emails.
This results in comments getting "lost" when people don't notice
them until they're specifically sought out.

It is more important for comments to be noticeable than
other fields -- looking up notes, invoices, or emails is intentional,
whereas seeing comments is incidental. To increase visibility,
move them up above the fold.

Also includes minor stylistic changes.

@NoRePercussions
Copy link
Contributor

NoRePercussions commented Jan 31, 2026

Another concern. I will merge regardless since this isn't a regression, but would be nice to fix it in one PR:

Too much priority inversion in this area. The textbox is bright and noticeable even though it's always there. The comments are gray and blend in with their container and neighbors, even though they should be one of the most eye-catching items on the page.

image

I sketched some alternatives. I'm partial to the third.

image image image

@BrilliantDeviation7
Copy link
Contributor Author

BrilliantDeviation7 commented Jan 31, 2026

Thanks for the feedback; I also prefer the 3rd one because it keeps the text input's background white.

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.

2 participants