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

Fix spotlight tabbed display and drop down list #65

Merged

Conversation

ammopt
Copy link
Member

@ammopt ammopt commented Jun 4, 2024

  1. Perform a search
  2. Scroll down to the bottom of the page. Click 'Create Spotlight'.
  3. Enter name, click 'Create Spotlight'
  4. Click "Edit" at the top.
  5. For each display types of the following:
  • Horizontal
  • Horizontal Carousel
  • Vertical
  • Single Title
  • Single Title with a Next Button
  • Text Only List
  1. Test both display modes:
  • Tabbed Display
  • Drop Down List
  1. Make sure you have at least 2 lists created at the bottom of the edit page.
  2. Verify that:
  • Switching tabs works as expected for all display types
  • Switching lists using the dropdown list works as expected for all display types

Before these patches:

  • 'Vertical', 'Single Title', 'Single Title with next button' and 'Text Only List' did not allow to switch from tab to tab.
  • When switching from the list dropdown it wouldn't change the list, but instead add to the one currently being viewed.
  • The 'active' tab on 'Tabbed display' was not styled correctly for all displays other than 'horizontal-carousel'

ammopt added 7 commits June 4, 2024 13:56
Vertical
Single Title
Single Title with a Next Button
Text Only
This can (should?) be extended to all common logic to further guarantee consistency across different spotlight displays and options, e.g.:
coverSize = medium | small
showRatings = 1 | 0
This fixes the 'active' styling not working correctly when viewing a tabbed collection spotlight of type other than 'horizontal-carousel'
The selector was too specific. The class '.titleScroller' is only
utilized in 'horizontal' display. By removing it from the
selector, changing list from the dropdown now works for all display types that are
not 'horizontal-carousel'
@Jacobomara901
Copy link

Patch looks good, seems to work as intended. As I said in the slack chat, I love the extra DRY implementations. We should definitely be making an effort to go out of our way to do this if we spot areas of improvement while we're working on nearby code

@Jacobomara901 Jacobomara901 merged commit c346bab into PTFS-Europe:ptfs_fix_spotlight_tabs Jun 5, 2024
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.

2 participants