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

Upgrade to Vue 3 and Vite #144

Merged
merged 29 commits into from
Aug 25, 2023
Merged

Upgrade to Vue 3 and Vite #144

merged 29 commits into from
Aug 25, 2023

Conversation

t11r
Copy link
Contributor

@t11r t11r commented Mar 16, 2023

Should be ready for release now.

- Add new sample manifest with fulltext and associated files
- Add API routes for locally handling annotation lists and annotations
- Update invalid files for testing
Upgrade to Vue 3 with Vite as build tool. Components keep the old options API, while the composition API is used in imported modules. Looks and functionality remain unchanged.

- Replace mixins with modules, using named imports
- Rename files and lint everything according to the latest Vue coding style
- Update all packages to their latest version, except OpenSeadragon
- Use Vitest instead of Jest for unit tests (Vue recommendation)
- Remove the last remnants of support for Internet Explorer
- Improve HTML templates
- Rewrite and extend tests
@t11r t11r marked this pull request as ready for review March 29, 2023 18:46
@t11r t11r requested review from ipf and paulpestov March 29, 2023 18:46
src/modules/iiif.js Fixed Show fixed Hide fixed
src/modules/iiif.js Fixed Show fixed Hide fixed
src/modules/iiif.js Fixed Show fixed Hide fixed
src/modules/iiif.js Fixed Show fixed Hide fixed
src/modules/iiif.js Fixed Show fixed Hide fixed
The actual error message is different on each browser, so make the test more generic.
We actually want version 3.0.0, not 3.1.0, which introduced a small but annoying bug.
@t11r t11r force-pushed the vue3 branch 4 times, most recently from 4bd0c3e to f926cb4 Compare April 27, 2023 12:35
@t11r t11r force-pushed the vue3 branch 6 times, most recently from 9f39a2a to 515341f Compare May 8, 2023 07:04
@ipf ipf force-pushed the vue3 branch 3 times, most recently from abdaae5 to e791938 Compare May 8, 2023 09:44
t11r added 3 commits May 17, 2023 22:36
- Enable display of multiple error messages
- Add ARIA attribute
- Allow configuration of the local IIIF server port so multiple servers can run at the same time
- Extend collections tests to check if multiple child manifests can be loaded successively
- Add basic tests for TIFY's API
t11r added 6 commits June 6, 2023 18:09
Update all packages to their latest minor version (except OpenSeadragon), remove unused packages.
Use ResizeObserver instead of an event handler. Not only does this make the code more concise, but also reacts to changes of the element size when the window size stays the same.
- Add close-all button
- Improve styling
@t11r t11r requested a review from paulpestov June 7, 2023 16:03
src/modules/filter.js Dismissed Show dismissed Hide dismissed
src/modules/filter.js Dismissed Show dismissed Hide dismissed
src/modules/filter.js Dismissed Show dismissed Hide dismissed
src/modules/filter.js Dismissed Show dismissed Hide dismissed
src/modules/filter.js Dismissed Show dismissed Hide dismissed
@t11r
Copy link
Contributor Author

t11r commented Jun 7, 2023

🎉

- Replace instance-dependent modules with globally available plugins to restore multi-instance functionality
- Move all functions from main to App for better testability
- Fix text flashing in fulltext panel
- Fix translations in help panel
- Improve coding style
- Improve unit tests
package.json Outdated
"eslint-plugin-vue": "^9.14.1",
"jsdom": "^21.1.2",
"openseadragon": "3.0.0",
"sass": "^1.62.1",
Copy link
Contributor

@paulpestov paulpestov Jun 19, 2023

Choose a reason for hiding this comment

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

Do we still need "sass" as dep?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah ok, somehow the build still succeeded for me when I tried to remove "sass".

@paulpestov
Copy link
Contributor

paulpestov commented Jun 19, 2023

Another issue I found is this:
davinci

With this error on the console:
TypeError: Cannot read properties of null (reading 'description') ...
Used IIIF manifest: https://iiif.ub.uni-leipzig.de/static/collections/toplevel.json

It seems that they use @label instead of label key. Tify should fail more gracefully here I think.

@t11r
Copy link
Contributor Author

t11r commented Jun 20, 2023

Good find, thanks! There are actually two bugs in the collection view, one regarding the label, another reading props from a manifest that is not loaded yet. Fix coming soon.

@t11r
Copy link
Contributor Author

t11r commented Jun 20, 2023

@label is invalid IIIF, see https://presentation-validator.iiif.io/validate?version=2.1&url=https://iiif.ub.uni-leipzig.de/static/collections/misc/cdvost2018.json. I would suggest to add "No label" or similar to the button, but not to show the invalid @label, and make sure the missing label does not break the whole app.

@paulpestov
Copy link
Contributor

Maybe give it a default label like "Manifest 1", "Manifest 2" and only if something useful (label/description) exists in the response, overwrite it. This could also work for collection labels.

@t11r
Copy link
Contributor Author

t11r commented Jun 21, 2023

Too much code just for displaying an invalid manifest. I’m just making sure the app does not break.

t11r added 3 commits June 21, 2023 12:59
Although invalid IIIF, there are collection manifests in the wild with missing labels, and we don’t want them to make TIFY crash.
@paulpestov
Copy link
Contributor

looks ok to me

Update all dependencies to their latest minor version, except OpenSeadragon.

Remove obsolete packages:
- sass
- vitest-canvas-mock
@t11r t11r merged commit 0a6fb24 into main Aug 25, 2023
9 checks passed
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.

2 participants