Media: Use react-photo-album for media library
#10737
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reverts commit a6b7e10.
swissspidy
added
Type: Enhancement
|
When this PR, can you ensure that highlight the difference from the original PR so this PR can be reviewed. |
|
There are no differences right now, any new changes can be seen in the commit messages and will of course be reflected in the PR description, as per usual. |
|
See #10463 (comment) |
|
Size Change: +2.52 kB (0%) Total Size: 2.75 MB
ℹ️ View Unchanged
|
|
Plugin builds for 4fe0cec are ready 🛎️!
|
|
@swissspidy I think the relevant tests should be fixed now. There's a failing e2e run but seems unrelated: https://github.com/GoogleForCreators/web-stories-wp/runs/5408325743?check_suite_focus=true I reviewed the snapshots and things seem generally fine but there are some slight changes -- might be good if you could double check to see if these seem expected. Also, please review the new file changes -- it's your PR so I can't request a review from you :) |
miina
reviewed
Mar 3, 2022
| @@ -72,6 +72,7 @@ describe('Inserting WebM Video', () => { | |||
| uploadedFiles.push(fileName); | |||
|
|
|||
| await page.waitForSelector('[data-testid="mediaElement-video"]'); | |||
| await page.waitForTimeout(100); | |||
There was a problem hiding this comment.
Note: it seems like without a small delay the correct video won't be available on time.
It's a bit hard to see what exactly is happening there since in interactive mode everything works as expected but I guess the new video will be otherwise added to the library after the click has happened already and that messes things up.
Not sure if there's a better way to fix these tests. Feel free to amend to the PR if you have ideas.
There was a problem hiding this comment.
One alternative could be clicking in the corner of the element to avoid the Insertion menu (and insert whatever element gets clicked on first) but not sure if that's the right approach either -- if we're waiting for the uploaded video.
There was a problem hiding this comment.
Ideally we'd use a more targeted waitFor... specifically for this new element.
The await page.waitForSelector('[data-testid="mediaElement-video"]')above is a bit pointless, as it is too generic and just waits for any video to exist in the library, which is always the case.
I'll try to find a more specific selector.
7 tasks
spacedmonkey
approved these changes
Mar 4, 2022
swissspidy
changed the title
react-photo-albumreact-photo-album for media library
8 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context
This re-introduces #10463 which was reverted in #10735 due to failing tests.
See those PRs for full context.
Previously react-photo-gallery was used to display media elements in the library pane, it has not seen any change since 1 July 2019. Therefore it was stuck at React version
^16.8.0which will cause 2 versions of React to be installed if are packages published.Summary
Replaces
react-photo-gallerywithreact-photo-albumin the media library.Relevant Technical Choices
To-do
User-facing changes
None
Testing Instructions
This PR can be tested by following these steps:
Reviews
Does this PR have a security-related impact?
No
Does this PR change what data or activity we track or use?
No
Does this PR have a legal-related impact?
No
Checklist
Type: XYZlabel to the PRFixes #10444