refactor: adopt primer design system ui components

[setchy]

trafficstars

Still WIP - raising early for visibility

Closes: #1541

Adopts the excellent GitHub Primer Design System Component Library - https://primer.style/components to replace our custom components and provide a level-up on user experience and visual consistency.

This PR, though sizeable, focuses on replacing the following components

• Icons
• Buttons
• Layout via Stack

I have also replaced most tests to use data-testid and getByTestId so that they're more resilient to UI changes

Items found during testing that should be resolved

• [ ] Horizontal scroll bar showing on notification animation (mark as read, etc)
• [ ] Different uses of colors (tailwind vs design token), eg: green color
• [ ] position of repository title tooltip gets cut off

[bmulholland]

I have also replaced most tests to use data-testid and getByTestId so that they're more resilient to UI changes

Lovely, nice touch.

[setchy]

@afonsojramos @bmulholland - I'm pretty happy with the completeness of this initial uplift to use the GitHub Primer UI Components.

I plan to update this issue later with before/after screenshots, but while that's pending, if you do happen to have a few minutes I'd appreciate your feedback by running this branch locally 🙏

[bmulholland]

Sure, testing this locally now :)

[bmulholland]

A minor nitpick: there's now two different versions of green. The Gitify logo in the tray, when there's notifs, is one -- I think the same as open PR green? The sidebar icons are another, along with the sub-icons for a notification.

[setchy]

A minor nitpick: there's now two different versions of green. The Gitify logo in the tray, when there's notifs, is one -- I think the same as open PR green? The sidebar icons are another, along with the sub-icons for a notification.

Good feedback.

The sidebar icons are now using the GitHub design tokens (colors), while the others (notification type icon and tray icons) are unchanged.

Post this PR I do intend to use the GitHub design token colors more extensively and reduce the amount of tailwind colors we're using

[bmulholland]

Yeah I think temporary mismatches are fine, too.

[setchy]

During further testing, noticed that when a notification row is animating out (mark as read, etc) a horizontal scroll bar is being shown temporarily. Added as an open task to the OP

[bmulholland]

I think open in background is broken on this branch

[setchy]

I think open in background is broken on this branch

Just tested and it's working for me...

[bmulholland]

Will do some more testing

[setchy]

How has the testing been going @bmulholland ? For me it's been really stable.

Once this is merged I plan to (finally) move forward with https://github.com/gitify-app/gitify/issues/985 - I have configured this on the atlassify codebase so should be an easy port across

[bmulholland]

Yeah it's been working well for me :)

[setchy]

@afonsojramos - I know you're a busy man atm, but if you do happen to have time to shake this down would love your feedback, too

[afonsojramos]

Giving it a try! 👀

[afonsojramos]

Wow, this has been a huge amount of work! Everything looks great!

However, I've found a couple of visual issues:

1. Input component hover border is red by default?

2. Spacing in some icons under accounts seems too tight. I imagine my name is too long 😅 If it was my full name it would be even worse 🧨

3. This weird hover in this text and icons under Accounts.

And got a few questions regarding implementation on some things. I know that to add primer/react we needed to add styled-components, but I don't think we should mix and match both styled-components and tailwind. So I'd probably change those sx's.

I'm also (sometimes) seeing some errors on the console, but I think that is related to this: https://styled-components.com/docs/faqs#shouldforwardprop-is-no-longer-provided-by-default

[setchy]

Appreciate the kind words @afonsojramos and the feedback on some visual inconsistencies. I'll address them shortly 👨‍💻

[setchy]

However, I've found a couple of visual issues:

Apologies this took a while to get back to. Items 1, 2 and 3 have been addressed

I think we're getting quite close to this being merged. 6.x.x milestone perhaps?

[afonsojramos]

@setchy do you want some help in the replacement of the sxs?

[setchy]

@setchy do you want some help in the replacement of the sxs?

Absolutely! Always welcome a friendly hand.

I haven't attempted to remove the style components used with primer components... I'm not sure how feasible that will be, but if possible I'm keen

[afonsojramos]

Seems like it is not possible 😅 We need to override the component's defaults... Oh well 🤷 It's annoying, but isn't much of a problem.

[setchy]

Thanks for taking a look 🙏

[setchy]

@bmulholland @afonsojramos - I've resolved all merge conflicts and done some final refactoring to our tailwind colors.

I think I'm ready for final review from you both if you have some time 🙏

There is one niggling issue remaining whereby the user-selected theme is not correctly loading on startup...

[setchy]

There is one niggling issue remaining whereby the user-selected theme is not correctly loading on startup...

This is now resolved. Left a few TODO comments for future us to clean up some of the legacy theming code

[setchy]

I'm going to merge this to main and it's getting tricky to work on other PRs with this large changeset pending.

We can always adjust any minor things as we go