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

Add infinite scrolling to alphabetical index #1624

Merged
merged 5 commits into from
Apr 24, 2024

Conversation

UnniKohonen
Copy link
Contributor

@UnniKohonen UnniKohonen commented Apr 18, 2024

Reasons for creating this PR

Alphabetical index does not load more concepts as it's scrolled. This PR adds functionality to address this.

Link to relevant issue(s), if any

Description of the changes in this PR

  • Add an event listener for scrolling alphabetical list
  • Detect when scrolling reaches the bottom of the list and load more concepts
  • Add messages for loading content to SKOSMOS variable
  • Add spinners when loading content

Addresses requirement 2 in #1563

Known problems or uncertainties in this PR

  • There could still be some bugs with loading (i.e. loading happening in the wrong order, concepts being loaded twice, some offsets being skipped, etc).

Checklist

  • phpUnit tests pass locally with my changes
  • I have added tests that show that the new code works, or tests are not relevant for this PR (e.g. only HTML/CSS changes)
  • The PR doesn't reduce accessibility of the front-end code (e.g. tab focus, scaling to different resolutions, use of .sr-only class, color contrast)
  • The PR doesn't introduce unintended code changes (e.g. empty lines or useless reindentation)

@UnniKohonen UnniKohonen added this to the 3.0 milestone Apr 18, 2024
@UnniKohonen UnniKohonen requested a review from joelit April 18, 2024 10:01
@UnniKohonen UnniKohonen self-assigned this Apr 18, 2024
<div class="sidebar-list" :style="getListStyle()" ref="list">
<template v-if="loadingConcepts">
<div>
Loading...
Copy link
Contributor

Choose a reason for hiding this comment

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

This could be a value from the window.SKOSMOS object, e.g. "Ladataan sisältöä", "Laddar mera resultat" & "Loading more items". Also if you want to use a spinner, something like <i class="fa-solid fa-spinner fa-spin-pulse"></i> could be considered.

Copy link
Member

Choose a reason for hiding this comment

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

Not sure if it's worth adding more translations to the SKOSMOS object, because the translation mechanism for the frontend may still change to something completely different that doesn't rely on the SKOSMOS object (e.g. reading the translations directly from a JSON file). See #1525 and #1523 (comment) for some thoughts

@@ -147,9 +194,12 @@ tabAlphaApp.component('tab-alpha', {
:href="getConceptURL(concept.uri)" @click="loadConcept($event, concept.uri)"
>{{ concept.prefLabel }}</a>
</li>
<template v-if="loadingMoreConcepts">
Loading...
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here as on L183

Copy link
Contributor

@joelit joelit left a comment

Choose a reason for hiding this comment

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

It works well now! A minor thing, but I'd get rid of the hardcoded loading message at this point already.

Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@UnniKohonen UnniKohonen requested a review from joelit April 18, 2024 12:38
Copy link
Contributor

@joelit joelit left a comment

Choose a reason for hiding this comment

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

LGTM!

@UnniKohonen UnniKohonen merged commit 06107d5 into main Apr 24, 2024
10 checks passed
@UnniKohonen UnniKohonen deleted the alphabetical-index-infinite-scroll branch April 24, 2024 07:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done (verified in test.dev.finto.fi, set Milestone 3.0 for both issue & PR)
Development

Successfully merging this pull request may close these issues.

3 participants