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

Restore date filtering functionality to publishers and subjects pages. #5980

Merged

Conversation

agmckee
Copy link
Contributor

@agmckee agmckee commented Dec 16, 2021

Closes #5955

Fix for UI regression

Technical

Largely inspired by the original functionality that got lost at some point, this restores the plotselected and plotclick event handlers but this uses events for communication between the graph module and the Carousel as the latter module has changed significantly since this functionality last existed in the codebase. The changes to Carousel should only come into effect when used on a publisher or subject page and should not affect it in any other way.

Testing

Fixes the UI regression described in #5955. Test filtering by clicking a year or dragging across a range of years, the Carousel should update with a suitable list of works from the selected date range or year.

Screenshot

filtered-1900-to-1910
filtered-2000-to-2010
unfiltered

Stakeholders

@mekarpeles mekarpeles added the Priority: 2 Important, as time permits. [managed] label Dec 20, 2021
@mekarpeles
Copy link
Member

@agmckee this is a fantastic effort to fix a long-standing regression. Thank you for putting so much effort into it! This code doesn't appear to leak into any other functionality on the site which should help with the review / make it low risk, cc: @jimchamp

Copy link
Collaborator

@jimchamp jimchamp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great! The code lgtm, and everything seems to be working properly. Thanks, @agmckee!

@jimchamp jimchamp merged commit db2d35a into internetarchive:master Jan 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: 2 Important, as time permits. [managed]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Publication range feature on publishers page not filtering as described
3 participants