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 loading spinners to Vue components #1636

Merged
merged 10 commits into from
Aug 2, 2024
Merged

Add loading spinners to Vue components #1636

merged 10 commits into from
Aug 2, 2024

Conversation

UnniKohonen
Copy link
Contributor

Reasons for creating this PR

Many Vue components do not currently have loading spinners, this PR adds them.

Link to relevant issue(s), if any

Description of the changes in this PR

Adds Font Awesome spinner icons for loading:

  • concept mappings
  • hierarchical tree
  • new concepts into hierarchical tree
  • vocab and term counts on vocab page

Addresses requirement 3j in #1521

Known problems or uncertainties in this PR

No loading texts are added in this PR

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 Jul 23, 2024
@UnniKohonen UnniKohonen self-assigned this Jul 23, 2024
@UnniKohonen UnniKohonen changed the title Add spinners to Vue components Add loading spinners to Vue components Jul 23, 2024
Copy link
Member

@osma osma left a comment

Choose a reason for hiding this comment

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

I tested this briefly and found a few issues within the code (see comments).

Also in the functionality, generally things are working well but there are a few issues:

Vocabulary home page

On the vocabulary home page (on my system, http://localhost/Skosmos/yso/en/ ), there is now a mappings box with a spinner that just spins forever:

image

The same happens on a concept page which doesn't have any mappings. For example the KANTO/FINAF page for myself http://localhost/Skosmos/finaf/fi/page/000152600 looks like this:

image

Possibly related to these, I can see this errors in the JS console (Firefox) on the vocab home page:

[Vue warn]: Extraneous non-props attributes (hasCounts) were passed to component but could not be automatically inherited because component renders fragment or text root nodes. 
  at <ResourceCounts key=0 concepts= 
Object { class: "http://www.w3.org/2004/02/skos/core#Concept", label: "Concept", count: 32423, deprecatedCount: 1650 }
 subTypes= 
Array(3) [ {…}, {…}, {…} ]
  ... > 
  at <App>

The FINAF concept page shows these errors in the JS console:

[Vue warn]: Unhandled error during execution of mounted hook 
  at <TabHierWrapper hierarchy= 
Array []
 selectedConcept="http://urn.fi/URN:NBN:fi:au:finaf:000152600" loadingChildren= 
Array []
  ... > 
  at <App>

and

Uncaught (in promise) TypeError: selected is undefined
    mounted http://localhost/Skosmos/resource/js/tab-hierarchy.js:211
    VueJS 6

Can you please try fixing these and then I can test it again?

resource/js/concept-mappings.js Show resolved Hide resolved
resource/js/term-counts.js Outdated Show resolved Hide resolved
resource/js/term-counts.js Outdated Show resolved Hide resolved
resource/js/term-counts.js Outdated Show resolved Hide resolved
@UnniKohonen UnniKohonen requested a review from osma August 1, 2024 11:20
Copy link
Member

@osma osma left a comment

Choose a reason for hiding this comment

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

All the problems I reported earlier seem to be fixed now. Great work!

Do you think it would be possible to add Cypress tests for the spinners?

Other than that, I think this can be merged.

Copy link

sonarqubecloud bot commented Aug 2, 2024

@UnniKohonen UnniKohonen merged commit 0601f71 into main Aug 2, 2024
10 checks passed
@UnniKohonen UnniKohonen deleted the add-spinners branch August 2, 2024 10:53
@osma osma modified the milestones: 3.x, 3.0 Jan 22, 2025
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.

2 participants