Skip to content

Fix Magic Select binding for Editions DataTable#9888

Merged
mekarpeles merged 2 commits intointernetarchive:masterfrom
schu96:9853/fix/magic-select-edition-table
Apr 15, 2025
Merged

Fix Magic Select binding for Editions DataTable#9888
mekarpeles merged 2 commits intointernetarchive:masterfrom
schu96:9853/fix/magic-select-edition-table

Conversation

@schu96
Copy link
Contributor

@schu96 schu96 commented Sep 18, 2024

Addresses #9853 Task 5

Fix

Technical

Initializes the ILE/SelectionManager classes before the Editions DataTable to allow the ILE to bind the click events and add class names to the relevant DOM elements.

Prevents the Editions DataTable from re-initialization with .isDataTable method.

Utilizes the drawCallback option for DataTables to remove visual inconsistencies when a user selects editions from multiple pages, clicks on clear selection, and re-navigates to a page that had a previously selected edition by checking the sessionStorage object.

Testing

Sign in with the admin account and navigate to a work that has >3 editions (the more the better).

Verify that the magic select editions beyond the first page of the Editions Table.

Press the Clear Selection button within the magic select toolbar on the bottom of the page.

Navigate back to the pages where the editions were previously selected. The magic select classes and styling should not be present anymore.

Screenshot

2024-09-18.12-05-56.mp4

Stakeholders

@cdrini

@schu96 schu96 marked this pull request as ready for review September 18, 2024 19:10
@schu96 schu96 changed the title Change init order of ILE and editions datatable for magic select Fix Magic Select binding for Editions DataTable Sep 18, 2024
@cdrini cdrini self-assigned this Sep 23, 2024
@mekarpeles mekarpeles self-assigned this Jan 21, 2025
@mekarpeles
Copy link
Member

@schu96 this looks like good to me, just needs a rebase please!

@schu96 schu96 force-pushed the 9853/fix/magic-select-edition-table branch from f816e52 to d6e9d58 Compare February 4, 2025 19:00
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 0% with 31 lines in your changes missing coverage. Please review.

Project coverage is 17.43%. Comparing base (fb469a6) to head (d6e9d58).
Report is 51 commits behind head on master.

Files with missing lines Patch % Lines
...ary/plugins/openlibrary/js/editions-table/index.js 0.00% 18 Missing and 7 partials ⚠️
openlibrary/plugins/openlibrary/js/index.js 0.00% 4 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #9888      +/-   ##
==========================================
- Coverage   17.50%   17.43%   -0.07%     
==========================================
  Files          87       87              
  Lines        4793     4811      +18     
  Branches      852      859       +7     
==========================================
  Hits          839      839              
- Misses       3432     3444      +12     
- Partials      522      528       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jimchamp jimchamp added the Needs: Response Issues which require feedback from lead label Feb 5, 2025
@mekarpeles mekarpeles merged commit 68ed6e7 into internetarchive:master Apr 15, 2025
4 checks passed
@mekarpeles mekarpeles removed the Needs: Response Issues which require feedback from lead label Apr 15, 2025
@jimchamp jimchamp mentioned this pull request Apr 16, 2025
Amara777 pushed a commit to Amara777/openlibrary that referenced this pull request Apr 23, 2025
* Change initialization order of ILE and editions datatable for magic select
* Fix edge case for magic select interaction with edition table
Comment on lines +326 to +335
// Import ile then the datatable to apply clickable classes to all listed editions
if (document.getElementsByClassName('editions-table--progressively-enhanced').length) {
import(/* webpackChunkName: "editions-table" */ './editions-table')
.then(module => module.initEditionsTable())
}
}
// conditionally load functionality based on what's in the page
if (document.getElementsByClassName('editions-table--progressively-enhanced').length) {
import(/* webpackChunkName: "editions-table" */ './editions-table')
.then(module => module.initEditionsTable());
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is the table being initialized twice?

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.

5 participants