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

Follow+Notes: Implement Grant Followers modal component #3407

Open
Tracked by #2960
TylerHendrickson opened this issue Aug 16, 2024 · 13 comments
Open
Tracked by #2960

Follow+Notes: Implement Grant Followers modal component #3407

TylerHendrickson opened this issue Aug 16, 2024 · 13 comments
Assignees
Labels
collaboration good first issue Good for newcomers Grant Finder Issues related to the Grant Finder javascript Pull requests that update Javascript code

Comments

@TylerHendrickson
Copy link
Member

TylerHendrickson commented Aug 16, 2024

Subtask of [STORY]: Update 'Status' to 'Follow + Note' feature #2960

Blocked by

Blocks

N/A

Definition of Done

Modal Design Link

  • Create a new "Grant Followers" modal component that provides the following:
    • A header of 'Grant Followers'
    • A list of all followers sorted alphabetically in ascending order by name.
    • User Avatar, User Name, User Team Name, User Email Address, date followed (this will show "Today", "[#] days ago" up to seven days, and more than seven days will show the date followed in the format "[Month] [Date]", i.e. May 4)
    • To the right of each follower's name will be a "Copy Email" button. Clicking the button will copy the email address for the individual user to clipboard
    • At the bottom of the modal there will be a button that allows users to copy all emails to their clipboard. (This should copy as a comma separated list of emails.)
    • When the user scrolls to the bottom of the modal list, a button should allow the ability to fetch the next page of followers for the grant.
      • If the request for the next page returns no results, the button should be hidden until the modal is relaunched.
  • Update the "Grant Activity" component so that the follower information text ("Followed by {{Follower Name}} and {{Count Followers - 1}} others") is hyperlinked so that when clicked, the new "Grant Followers" modal displays.

Implementation details

  • The GET /:grantId/followers route implemented in Follow+Notes: Expose ability to retrieve grant followers via Finder API #3399 supports requests to fetch (paginated) followers of a grant within the same organization as the currently-authenticated user. This endpoint should provide all data necessary to populate this modal (name, email, team name of following users, plus timestamp of the follow).
  • For "Copy Email" and "Copy all Emails" buttons, use the CopyButton component defined in packages/client/src/components/CopyButton.vue.
    • Ideally, this component should be updated so that its core functionality may be reused while also being styled as a button (either filled or unfilled) in a manner consistent with the Figma designs linked above.
  • See existing User Avatar Component
@TylerHendrickson
Copy link
Member Author

TylerHendrickson commented Aug 17, 2024

@caitlinwinner I took the liberty of prescribing behavior for paginating followers within the modal:

  • When the user scrolls to the bottom of the modal list, a button should allow the ability to fetch the next page of followers for the grant.
    • If the request for the next page returns no results, the button should be replaced by "No more followers" until the modal is relaunched.

If you think that the described behavior sounds good, can we get update the designs accordingly? Otherwise we can discuss!

@TylerHendrickson
Copy link
Member Author

@caitlinwinner Another discussion topic: The current designs show "Copy Email" and "Copy All Emails" buttons that look different fro our existing copy button designs (e.g. the "📎 Copy Link" button or what displays next to email addresses in the "Team Status" card on the Grant Details view).

Proposal:

  1. Update designs to drop the "Copy Email" button to the right of each follower and update the displayed email address to use the same thing that we use on the "Team Status" card.
  2. Update the design to provide a "Copy All Emails" link that is closer in look-and-feel to the existing copy buttons?

@caitlinwinner
Copy link

@TylerHendrickson I don't actually love the copy email behavior that exists in-line in status components. I don't think its super obvious and I would be open to just using plain text for emails here if metrics show what I expect (not much usage). This minimal treatment was a workaround due to limited real estate next to the avatars in status units.

Since we have extra space in the modals I think it's advantageous to use buttons as designed, even though it is not 100% aligned with how emails are copied elsewhere in the app.

@caitlinwinner
Copy link

@TylerHendrickson re: pagination in the modal, that sounds okay by me. @samserif23 what do you think? Since there is a sticky button at the bottom of the modal we might want to avoid stacking buttons and use a text link for "show more".

Second thought, we may not need the "no more followers" text, could instead just hide the "show more" link when there are no more to show.

@ClaireValdivia ClaireValdivia added the good first issue Good for newcomers label Aug 27, 2024
@greg-adams greg-adams self-assigned this Sep 11, 2024
@greg-adams
Copy link
Contributor

@ClaireValdivia Should the current user be excluded from the list of followers (if they follow it) in this modal?

@ClaireValdivia
Copy link
Contributor

@greg-adams let's keep the current user listed on the modal.

cc @caitlinwinner for awareness if we want to think on something for the future that indicates on the modal if the current user is following. My reasoning to keep the current user on the modal is that I think it could be confusing for the user to be omitted unless we have some other kind of indicator that they are following, and I can't think of much of a downside of having them listed (except for there not being a good reason for them to copy their own email).

@caitlinwinner
Copy link

agree

@ClaireValdivia
Copy link
Contributor

@greg-adams This looks so good! Some notes as I'm testing in staging:

  • All avatars are showing the same color (purple) even when the avatar has been set to a different color
  • When the name or team name is long, the display is a bit wonky. checking in with design on how we want to handle this
    image

@TylerHendrickson
Copy link
Member Author

@ClaireValdivia @greg-adams Regarding

  • When the name or team name is long, the display is a bit wonky. checking in with design on how we want to handle this

The user "headline" design implemented in this modal is very similar to the designs for the notes feed; the spec for the notes feed provided in #2960 (eng subtask #3428) says

Each note will show: the user's avatar, user name, user's team name (truncated if it reaches the enter of one line), the user's email, full text of the note which will wrap text, and the date the note was posted or edited (whichever is most recent). The date will show "Today", "[#] days ago" up to seven days, and more than seven days will show the date followed in the format "[Month] [Date]", i.e. May 4

So perhaps it makes sense to observe the same guidelines for addressing the issue @ClaireValdivia noted above for the Followers modal.

Additionally, @greg-adams Do you think it would be worth creating a reusable component for this kind of thing, which could be used in the modal as well as the notes feed? I'm also noticing that the Figma designs use this same heading for items in the Share sidebar block on the Grant Details page (#3240).

@TylerHendrickson
Copy link
Member Author

TylerHendrickson commented Sep 20, 2024

@greg-adams Also, regarding

  • All avatars are showing the same color (purple) even when the avatar has been set to a different color

FYI, the UserAvatar component has a color property, and we store users' preferences for this in the users.avatar_color DB column. It occurs to me that this isn't represented in the API response for paginating followers; rather than retrieving the avatar color from a different API call, I think we should just update getFollowersForGrant() to include that column in the result set.

Probably also makes sense to do the same for getOrganizationNotesForGrant(). I'll make an edit on #3428 to capture this change.

@ClaireValdivia
Copy link
Contributor

@TylerHendrickson @greg-adams yes, let's match the Notes feed so that the first line is truncated to one line (and therefore the copy email button as a fixed height and width)

@greg-adams would it be helpful if I create two additional issues - one for truncating, and one for the avatar color, or would you prefer to continue to work off this issue?

@greg-adams
Copy link
Contributor

Thanks all, I've got these updated
@TylerHendrickson I found what I needed to add in the avatar color, and I added a separate component to reuse this header text.
@ClaireValdivia no need for any additional issues, I put all these changes in my open PR for #3428

@ClaireValdivia
Copy link
Contributor

Retested and this is all working as expected, thanks Greg!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
collaboration good first issue Good for newcomers Grant Finder Issues related to the Grant Finder javascript Pull requests that update Javascript code
Projects
Status: 🚢 Completed
Development

No branches or pull requests

4 participants