usdigitalresponse / usdr-gost

USDR-hosted grants management tools
http://grants.usdigitalresponse.org
Apache License 2.0
32 stars 21 forks source link

Follow+Notes: Implement Grant Followers modal component #3407

Open TylerHendrickson opened 3 months ago

TylerHendrickson commented 3 months ago

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

Blocked by

Blocks

N/A

Definition of Done

Modal Design Link

Implementation details

TylerHendrickson commented 3 months ago

@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 commented 2 months ago

@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 commented 2 months ago

@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 commented 2 months ago

@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.

greg-adams commented 2 months ago

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

ClaireValdivia commented 2 months ago

@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 commented 2 months ago

agree

ClaireValdivia commented 1 month ago

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

TylerHendrickson commented 1 month ago

@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 commented 1 month ago

@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 commented 1 month ago

@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 commented 1 month ago

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 commented 1 month ago

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