Filled vs Unfilled Bookmarks

[ChristopherChudzicki]

What are the relevant tickets?

https://github.com/mitodl/hq/issues/4712

Description (What does it do?)

Makes userlist bookmark and learningpath icon filled vs unfilled based on membership in at least 1 list.

Screenshots (if appropriate):

Cards

List Cards

List Cards, Mobile

How can this be tested?

1. As a logged in user, click the bookmark button on a card to change its membership in "userlists".
   • try this throughout the app, particularly for horizontal "ListCard" (search page) and vertical cards
2. When the resource belongs to at least one list, the bookmark should be filled.
3. When the resource belongs to zero lists, the bookmark should be unfilled.
4. The stafflist button should behave similarly.
5. Try adding/removing featured courses (homepage "Featured Courses" carousel) to a userlist.
   • in contrast to RC, the carousel resources should maintain their order.

[ChristopherChudzicki]

Regarding updateListParentsOnAdd and updateListParentsOnDestroy :

1. The featured carousel on the homepage is populated by an API that has an intentionally randomized order.
2. In general throughout the app, when data might change, we tell react query "invalidate the cached data" and it re-fetches.
3. Adding the featured courses to a userlist changes their data (it affects `resource.user_list_parents). However, if we invalidate the cache and refetch new, updated data, then the carousel order will change, which is strange.
   • you can see this behavior on RC / Prod, but the bookmarks won't fill / unfill, so it's less noticeable that position has changed.
4. So instead of invalidating the cache and refetching the data, we manually update the data on the frontend.

We only do this for userlists, not for learnignpaths, because...

• changing a resource's learningpath membershould can change whether it is a featured course or not, so we really do need to refetch the data.

[abeglova]

You should add a little more padding around the list icon, it overlaps the learning format on smaller mobile screens (chrome iphone SE screen shown) this doesn't happen on main

[ChristopherChudzicki]

@abeglova One thing to keep in mind is that normal users only have a single icon—the list icon is only visible to staff.

But even for normal users, this exacerbates the overlap issue more than it should. I'll adjust a bit.

E.g.,

[abeglova]

This works fantastic generally and looks great! Some small fixes are needed for smaller mobile screens.

Good point that the issue is mostly for just admins, not normal users

[ChristopherChudzicki]

@abeglova I've tweaked the button positioning to account for button paddings. The cards should look good at 375px with one button, and may look good with 2 buttons depending on date length.

[abeglova]

lgtm

[ChristopherChudzicki]

pre-commit.ci run

[ChristopherChudzicki]

Since github says this is mergable, and pre-commit just says

timeout waiting for merge ref

I'm going to merge this.