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

External Media: Render Media after authentication #16044

Merged
merged 1 commit into from
Jun 4, 2020

Conversation

obenland
Copy link
Member

@obenland obenland commented Jun 2, 2020

Fixes a bug where the first query after authenticating with Google would show Albums instead of the most recent photos.

See #15717 (review)

Changes proposed in this Pull Request:

  • Swaps authentication variable to make state clearer.
  • Passes a authentication state update function to original components.
  • Authentication component updates auth state now instead of populating the modal.

Does this pull request change what data or activity we track or use?

No.

Testing instructions:

  • Go through the Google Photos authentication flow
  • Make sure the flow fires when there is no connection
  • Make sure it doesn't as for authentication when Google Photos is already authenticated
  • Check that after authenticating the initial results are recent photos

Proposed changelog entry for your changes:

None needed.

@obenland obenland added [Type] Bug When a feature is broken and / or not performing as intended [Status] Needs Team Review [Extension] External Media Extend all block editor media tools to support external providers labels Jun 2, 2020
@obenland obenland requested a review from a team June 2, 2020 19:13
@obenland obenland self-assigned this Jun 2, 2020
@matticbot
Copy link
Contributor

Caution: This PR has changes that must be merged to WordPress.com
Hello obenland! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer, review, and approve D44311-code before merging this PR. Thank you!
This revision will be updated with each commit to this PR

@jetpackbot
Copy link

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

E2E results is available here (for debugging purposes): https://jetpack-e2e-dashboard.herokuapp.com/pr-16044

Scheduled Jetpack release: July 7, 2020.
Scheduled code freeze: June 30, 2020

Generated by 🚫 dangerJS against bbf728f

Copy link
Contributor

@marekhrabe marekhrabe left a comment

Choose a reason for hiding this comment

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

Since I had no albums in my google account, I've experienced the worst case version of this bug - seeing a blank screen with "nothing found" after connecting my account and no obvious way to do anything about it.

This fixes the problem and I'm able to see my photos right when the connection succeeds. 👍

@obenland obenland merged commit e3d759e into feature/external-media Jun 4, 2020
@obenland obenland deleted the fix/google-query-after-auth branch June 4, 2020 14:41
obenland added a commit that referenced this pull request Jun 25, 2020
* External Media: Render Media after authentication (#16044)

See #15717 (review)

* [not verified] Handle 401 responses (#16041)

See #15717 (review)

* External Media: Better reflect accepted media types (#16071)

* External Media: Account for multiple images (#16077)

See #16071 (comment)

* External Media: Fix Modal focus loss on arrow keypress (#16055)

Co-authored-by: Marek Hrabe <marekhrabe@me.com>

* External Media: Add API unit tests (#15988)

* [not verified] Add External media endpoint

* [not verified] Add unit tests

* [not verified] Add class coverage info

* Simplify schema

* Fix image copy test

* Use local image

* Remove duplicate file ending

Props @frontdevde.

* External Media: Improve Insert Flow (#16081)

* [not verified] # This is a combination of 5 commits.

External Media: Improve Insert Flow

See #15867

Remove unused import

Lock pointer events when copying
This prevents a UX issue where user can still interact with the modal content while the "Inserting..." request is pending.

Announce Inserting action along with alt text of selected images.

Update modal headers to reflect status

Props @jancavan.

* External Media: Fix mobile modal size (#16098)

* External Media: Improve Insert Flow

See #15867

* Remove unused import

* Lock pointer events when copying
This prevents a UX issue where user can still interact with the modal content while the "Inserting..." request is pending.

* Announce Inserting action along with alt text of selected images.

* Fix mobile mobile width to take up the full available modal space

* Change is-copying to modifier instead of element to follow BEM syntax

Co-authored-by: Konstantin Obenland <obenland@gmx.de>
Co-authored-by: Bart <bartlomiej.kalisz@gmail.com>

* External Media: Loading and disabled states for the copying state (#16103)

* disable pagination while loading

* fade out all images when copying

* disable event handlers and mark as disabled when copying

* disable pexels search when copying

* disabled google filters when copying

* add text to copying indicator

* disable google photos view selector when copying

* [not verified] remove debugging condition

* Pass functions not booleans to event emitters

Co-authored-by: Konstantin Obenland <obenland@gmx.de>

* Removed :not() copying css state since they are the same modal now

* Reset focus to the modal after clicking Insert. Otherwise focus is reset to the body and focus needs to stay trapped within the modal.

* Cosmetically rearranging code order

* External Media: Screen Reader accessible states for Insert flow (#16113)

* Add visually hidden description for the external media modal to give improved screen reader instructions and communicate the copying state

* [not verified] Remove title attribute from grid images

* [not verified] Move Inserting Image... copy to before the image so it is announced first.

* Separate-out description and element id

Helps with keeping describedby id in sync

Co-authored-by: Konstantin Obenland <obenland@gmx.de>

Co-authored-by: Jerry Jones <jones.jeremydavid@gmail.com>
Co-authored-by: Bart <bartlomiej.kalisz@gmail.com>
Co-authored-by: Marek Hrabe <marekhrabe@me.com>

* External Media: Restore focus on modal close (#16102)

* External Media: Disable for existing galleries (#16176)

* External Media: Disable for existing galleries
* Update comment

* External Media: Improve upload permission check (#16185)

* Use `value` to disable for existing galleries

`addToGallery` is not used by all gallery-type blocks, like tiled-gallery or slideshow.

Props @jeherve.

Co-authored-by: Bart Kalisz <bartlomiej.kalisz@gmail.com>
Co-authored-by: Marek Hrabe <marekhrabe@me.com>
Co-authored-by: Jerry Jones <jones.jeremydavid@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Extension] External Media Extend all block editor media tools to support external providers Touches WP.com Files [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants