Skip to content
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

Add profile hover cards to more screens #6453

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

bdbch
Copy link

@bdbch bdbch commented Nov 17, 2024

This PR adds ProfileHoverCards to the following screens:

  • Notifications (On the first author & on each authors names in an expanded notification)
  • Starter Pack profile list
  • Starter Pack wizard profile list

Specially inside the Starter Pack wizard it was really hard to quickly see who a person is because you also couldn't open a profile in a new tab from there. This should help to quickly see what person you're adding to your Starter Pack.

@bdbch
Copy link
Author

bdbch commented Nov 22, 2024

Looking forward for feedback on this. I'd love to see more profile cards in a few places (as described above).

@gaearon
Copy link
Collaborator

gaearon commented Nov 23, 2024

  • Notification: Makes sense, if this was a separate PR I'd just merge that in.
  • Starter Pack: Doesn't make sense to me, it's very duplicative. Would be better to extend the rows if we really need to.
  • Starter Pack Wizard: I'm not sure it's worth doing it there. It doesn't work very well because it listens to the wrong scroll target, so it doesn't dismiss on scroll. This would have to be fixed. Still, what we should fix there is opening in a new tab.

Copy link
Collaborator

@gaearon gaearon left a comment

Choose a reason for hiding this comment

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

let's do it just in notifications. and let's instead fix starter pack wizard to use links / open in new tab

@bdbch
Copy link
Author

bdbch commented Nov 23, 2024

Sure thing, will do. Should I look into linking for new tabs for starter packs in a new PR?

@gaearon
Copy link
Collaborator

gaearon commented Nov 24, 2024

Should I look into linking for new tabs for starter packs in a new PR?

Yea probably best to separate.

@bdbch
Copy link
Author

bdbch commented Nov 24, 2024

Reverted my changes made to the starter pack wizard & the profile card component, but want to fix the anchor for the profile card. Right now the position is a bit off. Will update when I'm done.

image

@bdbch
Copy link
Author

bdbch commented Nov 24, 2024

Done! 👍

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