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

Update channel topic browsing #8535

Conversation

marcellamaki
Copy link
Member

@marcellamaki marcellamaki commented Oct 25, 2021

Summary

Updates the channel topic page to use:

  • new cards and layout
  • adds "within channel" search functionality (like the Library page, but contained to current channel)
  • navigation between folders through sidebar/dropdown and breadcrumbs

References

Addresses #8306

Figma

topic-filtering

mobile-view

Reviewer guidance

Much of the search/filter functionality is copied from LibraryPage and while it deserves some testing to make sure everything is still working, it is more a question if there is some case that was unintentionally broken, rather than a full review of all of the code - since it continues to just build on Richard's original PR.

On the front end - there are some pieces that feel a little...janky, for lack of a better word, so if anyone has suggestions of how I can make things feel a little smoother, would be happy to hear them. Thinking particularly of the embedded side bar and the computed prop used for "stickiness"

@nucleogenesis would appreciate your eyes on the updates to the fullscreen side panel, and make sure that it still works with your plan and uses, and that I haven't caused any regressions there

What is included in this PR/What to test for - Manual/Design QA:

  • Accurate reflection of Figma specs (cc @jtamiace) - although not all metadata will be displayed
    • updated card designs
    • Overall layout
    • Buttons and tooltips
    • Responsive layout working as expected across browsers and screens
  • Filtering and keyword search as implemented by @rtibbles still working 😄 with same checklist as library page, although search results should be limited to the current channel:
    • Can search by keyword
    • Can search by single filter
    • Can search by multiple filters
    • Can view more
    • Can clear all
    • Can clear filters individually
    • Side panel accurately disables and/or hides filters and filter groups that are not available on search results

Testing checklist

  • Contributor has fully tested the PR manually
  • If there are any front-end changes, before/after screenshots are included
  • Critical user journeys are covered by Gherkin stories
  • Critical and brittle code paths are covered by unit tests

PR process

  • PR has the correct target branch and milestone
  • PR has 'needs review' or 'work-in-progress' label
  • If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
  • If this is an important user-facing change, PR or related issue has a 'changelog' label
  • If this includes an internal dependency change, a link to the diff is provided

Reviewer checklist

  • Automated test coverage is satisfactory
  • PR is fully functional
  • PR has been tested for accessibility regressions
  • External dependency files were updated if necessary (yarn and pip)
  • Documentation is updated
  • Contributor is in AUTHORS.md

…cards, rather than modify existing components, to prevent regressions
…t content bookmark status in LearningActivityBar
…cards, rather than modify existing components, to prevent regressions
…ch cards are used and grid at different sizes
…rking. No translations/string mapping - using ids
@marcellamaki marcellamaki changed the title DRAFT Update channel topic browsing Update channel topic browsing Oct 26, 2021
@marcellamaki marcellamaki marked this pull request as ready for review October 26, 2021 13:45
@marcellamaki
Copy link
Member Author

Superseded by #8548

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.

None yet

2 participants