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

Media: Hide edit button on private sites #17347

Merged
merged 1 commit into from
Aug 31, 2017

Conversation

jeremeylduvall
Copy link
Contributor

This is a temporary workaround to hide the Edit button on media items on private sites. It's related to #13625, which has received a few user reports. Right now, if you try to edit a media item in Calypso on a private site, you receive this error:

28747782-dc573464-746c-11e7-8dd7-105ac159b241

It's not clear why the edit action didn't work since you can edit media items in wp-admin.

My understanding (from this comment) is that this is a CORS limitation at the moment.

To test

  1. On a private site, open the following URL with the branch loaded: http://calypso.localhost:3000/media/
  2. Click on an item in your media library and then click the blue "Edit" button in the header

You should not see an edit button on the image. This should only affect private sites on WordPress.com.

Before

s9m9ew

After

screen shot 2017-08-18 at 1 43 05 pm

@jeremeylduvall jeremeylduvall added [Feature] Media The media screen in Calypso, general media management, or integration with third party media. [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Aug 18, 2017
@jeremeylduvall jeremeylduvall self-assigned this Aug 18, 2017
@matticbot matticbot added the [Size] S Small sized issue label Aug 18, 2017
@matticbot
Copy link
Contributor

@donnapep donnapep requested review from retrofox and gwwar August 21, 2017 11:07
@donnapep
Copy link
Contributor

donnapep commented Aug 21, 2017

It looks like this will affect VideoPress videos as well. I'm guessing the same error applies in that scenario?

Also, if I've previously edited an image, I still see the "Restore Original" button although the "Edit Image" button is indeed removed. The image does restore when the button is clicked, so maybe it's fine to still show it:

restore-original

@gwwar
Copy link
Contributor

gwwar commented Aug 21, 2017

My understanding (from this comment) is that this is a CORS limitation at the moment.

That's correct!

I still see the "Restore Original" button although the "Edit Image" button is indeed removed.

We want to update this eventually but it does require some endpoint work. At the moment restore, adds another copy of the original as the latest edit instead of pointing at the original image. In this case the client doesn't have enough information to detect an, edit -> restore case.

@gwwar
Copy link
Contributor

gwwar commented Aug 21, 2017

@retrofox do you recall if something changed recently? I thought we checked for this before.

Copy link
Contributor

@gwwar gwwar left a comment

Choose a reason for hiding this comment

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

@jeremeylduvall this tests pretty well for me, unless other folks had feedback feel free to :shipit:

@retrofox
Copy link
Contributor

@retrofox do you recall if something changed recently? I thought we checked for this before.

I'm not aware of any recent change, Kerry. :-(

@gwwar
Copy link
Contributor

gwwar commented Aug 30, 2017

@jeremeylduvall did you need help landing this one?

@jeremeylduvall
Copy link
Contributor Author

@gwwar no - sorry! Will get this landed first thing tomorrow morning.

This is a temporary workaround for #13625 so users no longer see the
edit button in Calypso on private sites since they can't edit a media
item at the moment. It prevents confusion because clicking the edit
button currently on a private site gives a weird error.
@jeremeylduvall jeremeylduvall force-pushed the fix/13625-hide-edit-button branch from 74c6a9a to fe40f4e Compare August 31, 2017 19:59
@jeremeylduvall jeremeylduvall merged commit e168cb7 into master Aug 31, 2017
@jeremeylduvall jeremeylduvall deleted the fix/13625-hide-edit-button branch August 31, 2017 21:49
@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Aug 31, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Media The media screen in Calypso, general media management, or integration with third party media. [Size] S Small sized issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants