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.

Sign up for GitHub

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

Jump to bottom

Media Library: improve uploading by URL section #11830

Merged
merged 3 commits into from
Mar 8, 2017
Merged

Media Library: improve uploading by URL section #11830

merged 3 commits into from
Mar 8, 2017

Conversation

retrofox
Copy link
Contributor

@retrofox retrofox commented Mar 7, 2017 •
edited
Loading

IMPORTANT

This PR is part of a set of PRs. Take a look to #11804, Pull requests section, to know to right order to review.

This PR is targeting to try/media-section-v2.

This PR implements some improvements in the uploading input field by URL of the media file:

uploading-by-url

Testing

  1. Go to the media library section: http://calypso.localhost:3000/media/<site>
  2. Open the Add by URL option

  1. Try to upload a valid file, an invalid one, passing an invalid URL (404), passing a value non-URL.

@retrofox retrofox 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 Mar 7, 2017
@matticbot
Copy link
Contributor

Test live: https://calypso.live/?branch=update/media-searching-bar

@matticbot matticbot added the [Size] M Medium sized issue label Mar 7, 2017
@retrofox retrofox changed the title Update/media searching bar Media Library: improve uploading by URL section Mar 7, 2017
@retrofox retrofox mentioned this pull request Mar 7, 2017
Closed
2 tasks
@retrofox retrofox requested review from jhnstn, gwwar and artpi March 7, 2017 12:30
@retrofox
Copy link
Contributor Author

retrofox commented Mar 7, 2017

I've removed the native input validation because it's annoying IMHO.

Once the error is triggered and the popover shown, it is not hidden after typing again.
Anyway, the commit is easily removable. 665219c

@iamtakashi
Copy link
Contributor

Design wise 👍

@matticbot
Copy link
Contributor

@retrofox This PR needs a rebase

@retrofox retrofox force-pushed the update/media-searching-bar branch from 665219c to b94d3a1 Compare March 7, 2017 23:58
@matticbot matticbot added [Size] S Small sized issue and removed [Size] M Medium sized issue labels Mar 7, 2017
@retrofox retrofox force-pushed the try/media-section-v2 branch from 66fce78 to 2474f28 Compare March 8, 2017 17:02
@matticbot
Copy link
Contributor

@retrofox This PR needs a rebase

@retrofox retrofox force-pushed the update/media-searching-bar branch from b94d3a1 to c085888 Compare March 8, 2017 20:18
@matticbot matticbot added the [Size] S Small sized issue label Mar 8, 2017
@retrofox retrofox force-pushed the try/media-section-v2 branch from 5d54642 to 98cdd8e Compare March 8, 2017 21:34
@retrofox retrofox force-pushed the update/media-searching-bar branch from c085888 to 197d8ae Compare March 8, 2017 21:40
@matticbot matticbot added the [Size] S Small sized issue label Mar 8, 2017
jhnstn
jhnstn approved these changes Mar 8, 2017
View reviewed changes
Copy link
Member

@jhnstn jhnstn left a comment

Choose a reason for hiding this comment

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

LGTM. works as expected. Tested Chrome, FF and Safari.

@retrofox
Copy link
Contributor Author

retrofox commented Mar 8, 2017

Thanks @jhnstn for the review 👍

@retrofox retrofox merged commit 2da0a29 into try/media-section-v2 Mar 8, 2017
@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 Mar 8, 2017
This was referenced Mar 9, 2017
Merged
Merged
@alisterscott alisterscott deleted the update/media-searching-bar branch October 16, 2017 03:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jhnstn jhnstn jhnstn approved these changes

@gwwar gwwar Awaiting requested review from gwwar

@artpi artpi Awaiting requested review from artpi

Assignees
No one assigned
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@retrofox @matticbot @iamtakashi @jhnstn