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

Automatically load rest of alphabetical index on vocabulary home page #1134

Merged
merged 1 commit into from
Mar 11, 2021

Conversation

osma
Copy link
Member

@osma osma commented Mar 10, 2021

This PR fixes #1108: it wasn't possible to scroll down to see the full alphabetical index of the first letter (typically A) on the vocabulary front page. The normal mechanism that triggers an AJAX request when scrolling down to the bottom of the first 250 results wasn't triggered on the vocabulary home page; it only worked on the alphabetical index page.

As analyzed in this comment the reason is that the callback for loading more lines isn't properly activated. The if check is too strict, it only works on the index page. I avoided the if check altogether and simply targeted the sidebar-grey-alpha CSS class, which is specific to the alphabetical index listings, instead of the more generic sidebar-grey class.

@osma osma added the bug label Mar 10, 2021
@osma osma added this to the 2.10 milestone Mar 10, 2021
@codecov
Copy link

codecov bot commented Mar 10, 2021

Codecov Report

Merging #1134 (281e00d) into master (a201ace) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1134   +/-   ##
=========================================
  Coverage     67.90%   67.90%           
  Complexity     1583     1583           
=========================================
  Files            32       32           
  Lines          3889     3889           
=========================================
  Hits           2641     2641           
  Misses         1248     1248           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a201ace...281e00d. Read the comment docs.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Collaborator

@kinow kinow left a comment

Choose a reason for hiding this comment

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

Tested with the containers from #962 , found no issues navigating the interface, scrolling, etc. Everything seemed to work fine.

@osma
Copy link
Member Author

osma commented Mar 11, 2021

Thanks for testing @kinow!

@osma osma merged commit 78dc205 into master Mar 11, 2021
@osma osma deleted the issue1108-fix-alphabetical-scrolling branch March 11, 2021 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Long lists not scrollable all way through
2 participants