-
Notifications
You must be signed in to change notification settings - Fork 94
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
Conversation
There was a problem hiding this 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:
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:
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?
There was a problem hiding this 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.
|
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:
Addresses requirement 3j in #1521
Known problems or uncertainties in this PR
No loading texts are added in this PR
Checklist
.sr-only
class, color contrast)