Skip to content

Conversation

@ifosli
Copy link
Contributor

@ifosli ifosli commented Sep 25, 2025

📲 What

Add a chevron to the project details cell in PPO. This also edits the alignment of the project details to match designs. (It was previously centered and now it's top-aligned.)

👀 See

Jira

Before 🐛 After 🦋
Screenshot 2025-09-25 at 11 31 13 AM Screenshot 2025-09-25 at 12 04 31 PM
Screenshot 2025-09-25 at 11 35 50 AM Screenshot 2025-09-25 at 12 07 22 PM

♿️ Accessibility

  • Voiceover is untested; I'll test this before launch.

✅ Acceptance criteria

  • Chevron looks good

@ifosli ifosli self-assigned this Sep 25, 2025
@ifosli ifosli marked this pull request as ready for review September 29, 2025 16:10
@ifosli ifosli requested review from a team, scottkicks and stevestreza-ksr and removed request for a team and scottkicks September 29, 2025 16:11
Copy link
Contributor

@stevestreza-ksr stevestreza-ksr left a comment

Choose a reason for hiding this comment

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

One minor thing left but otherwise no issues.

The chevron looks off to me being top-aligned, I would expect it to be centered relative to be centered relative to the rest of the stuff in project details. But that's a design thing. If that's what Alison wants then LGTM 👍

Comment on lines +56 to +58
.frame(width: Spacing.unit_04, height: Spacing.unit_04)
.padding(Spacing.unit_01)
.background(.clear)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Move these to PPOStyles

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, I actually don't think it makes sense to move these to PPOStyles in this case. The figma basically says the height and width of the icon should be 24 (ie unit 6 in the new spacing). However, the icon in the figma looks like it has built-in padding. To represent this, I shifted some of the icon width/height to be padding instead. I don't expect this to be true for the other icons (but I won't be sure until they're all added).

What do you think of leaving this as-is for now, and moving shared icon spacing into PPOStyles, if we end up actually having shared spacing?

@ifosli
Copy link
Contributor Author

ifosli commented Oct 1, 2025

One minor thing left but otherwise no issues.

The chevron looks off to me being top-aligned, I would expect it to be centered relative to be centered relative to the rest of the stuff in project details. But that's a design thing. If that's what Alison wants then LGTM 👍

Yeah, based on the v2 designs, I changed the whole project details cell to be top-aligned. I'll double-check with Alison or Tae before merging this, just in case these designs are out of date.

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