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

Update to React 18 #3864

Merged
merged 4 commits into from
Mar 14, 2024
Merged

Update to React 18 #3864

merged 4 commits into from
Mar 14, 2024

Conversation

cbeer
Copy link
Collaborator

@cbeer cbeer commented Dec 14, 2023

No description provided.

@cbeer cbeer force-pushed the mui5-react-18 branch 5 times, most recently from 122d8ae to 28488ec Compare December 14, 2023 18:38
@codecov-commenter
Copy link

codecov-commenter commented Dec 14, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (593e81b) 92.53% compared to head (fac2bad) 92.43%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3864      +/-   ##
==========================================
- Coverage   92.53%   92.43%   -0.11%     
==========================================
  Files         203      203              
  Lines        3579     3581       +2     
==========================================
- Hits         3312     3310       -2     
- Misses        267      271       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@daxid
Copy link

daxid commented Dec 22, 2023

Hi, I'm stopping by after a discussion on Slack IIIF.

We are putting some effort to upgrade the plugin here : https://gitlab.tetras-libre.fr/iiif/mirador/mirador-annotations

The strategy is to follow what's happening on the Mirador side. We first target an upgrade to React17 and MUI5 (against the corresponding mui5 branch of Mirador), then we will continue to React 18.

In the process we also target major UI improvements, video annotation support, etc.

Finally, our first investigations towards a React18 upgrade showed that the paper.js library that is used in mirador-annotations to draw and render the annotations may be a show stopper. Indeed, the library seems unmaintained since 2 years or so and the current version is recommended wit React17. We managed to make it work with React18.0 but not with the latest React18.2.

Anyway, I think that an in place replacement for paper.js would be very welcome, if not absolutely necessary. Any input on the matter is very welcome.

Base automatically changed from mui5 to master February 1, 2024 16:17
@cbeer cbeer marked this pull request as ready for review February 1, 2024 16:18
@d-flood
Copy link

d-flood commented Mar 8, 2024

Thanks for all the work on this, @cbeer!

I've tested this branch in a project we're (https://github.com/artshumrc) working on right now. I did not find any issue or regressions with the core Mirador viewer. Progress seems to be going well for this upgrade. With the below exceptions, it was essentially a drop-in replacement that worked.

I'm sure it is no surprise that I needed to disable our image-tools and annotations plugins in order to build and run this version.

It would be easier for us to test these upgrades in a real world project if there was a prelease distribution that we could include in our CI/CD pipeline. Perhaps I am missing something obvious, but the only way I found to test this branch in our project was to build from source myself and include the distribution in our project.

@cbeer
Copy link
Collaborator Author

cbeer commented Mar 14, 2024

Merging per Mirador Community Call 3/14

@cbeer cbeer merged commit 8e002b3 into master Mar 14, 2024
7 of 8 checks passed
@cbeer cbeer deleted the mui5-react-18 branch March 14, 2024 15:08
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.

4 participants