learning resource drawer list buttons #1537
Conversation
…urce drawer given a known authentication status
| }) | ||
|
|
||
| const section = screen | ||
| .getByRole("heading", { name: "Info" })! |
There was a problem hiding this comment.
| .getByRole("heading", { name: "Info" })! | |
| .getByRole("heading", { name: "Info" }) |
getBy will always be non-null.
Suggestion: I would just skip the non-null assertions all together and call invariant(section) if you need to refine the type. The error message will be better if it did happen to be null. Also, imo, we should add a linting rule forbidding the non-null assertions.
| Info | ||
| </Typography> | ||
| <ListButtonContainer> | ||
| {user?.is_authenticated && user?.is_learning_path_editor && ( |
There was a problem hiding this comment.
mentioning: Just user?.is_learning_path_editor should be fine
| {user?.is_authenticated && ( | ||
| <CardActionButton | ||
| filled={inUserList} | ||
| aria-label="Add to User List" |
There was a problem hiding this comment.
Probably best dealt with in a separate PR for all the cards at once, but we did have accessibility feedback that this label text is confusing, suggesting "Bookmark Resource(course?video?)" instead, or "Save..."
| // user info is still loading | ||
| return null | ||
| } | ||
| if (user.is_authenticated) { |
There was a problem hiding this comment.
Suggestion: drop the conditional above and
| if (user.is_authenticated) { | |
| if (user?.is_authenticated) { |
| const expectedButtons = | ||
| expectAddToLearningPathButton && expectAddToUserListButton ? 2 : 1 | ||
| expect(buttons).toHaveLength(expectedButtons) | ||
| if (expectAddToLearningPathButton) { |
There was a problem hiding this comment.
Suggestion: I like to do
expect(
!!within(section).queryByRole("button", {name: "Add to Learning Path" })
).toBe(expectAddToLearningPathButton)
because:
- no conditional => we're sure the assertion is always running
- currently this asserts presence in the truthy case. With the
queryByversion, it also asserts absence in the falsy case. (I guess you do that right now with the button count).
What are the relevant tickets?
Closes https://github.com/mitodl/hq/issues/5297
Description (What does it do?)
This PR adds "add to list" buttons to the learning resource drawer. When you bring up the drawer, if you are a learning path editor you will see both the add to learning path and user list buttons. If you are a normal user, you will only see the add to userlist button, just like how it works on cards.
Screenshots (if appropriate):
How can this be tested?
mit-learnon this branch