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

Add view button for file input fields in admin (w/ dynamic support) #650

Merged
merged 7 commits into from
Aug 19, 2016

Conversation

tripflex
Copy link
Collaborator

@tripflex tripflex commented Apr 7, 2016

Mike,
I know you mentioned the possible issues with this in the other closed issue, #631 ... I actually had already started working on this, and the only potential issue I see that you mentioned is a relative URL (but I have yet to see those anywhere).

Example
example

Basically this adds a View button that uses jQuery to get the URL from the input, and open it in a new window if the value has :// in it. If it does not, will add the file_no_url class to the input, then remove it after a second. That class has some CSS animation to flash the background the same red color as core WordPress error class (in admin notices).

I went ahead and used the same jQuery .live even though it's depreciated, just to try and keep everything as similar as possible.

If you don't want this in core that's fine, just let me know and I'll add it to my plugin ... just already had it completed for the most part for core before I saw your reply on the issue.

@mikejolley
Copy link
Contributor

Looks like it could be useful 👍 I would however avoid live() and use .on() - WPJM should be updated to follow suit, cc @georgestephanis

@tripflex
Copy link
Collaborator Author

@mikejolley no problem, I can go ahead and update the PR to use .on() instead (updating existing ones as well) ... let me know if you want me to and i'll get it taken care of today.

@tripflex
Copy link
Collaborator Author

@mikejolley lmk and i'll update this PR 👍

@georgestephanis
Copy link
Member

@tripflex ^^ just sent it as a PR in #664

@tripflex
Copy link
Collaborator Author

tripflex commented Apr 29, 2016

@mikejolley @georgestephanis Updated my PR to include the updates, as well as merge the update from your PR #664

Tested with updates and working correctly from my end

@tripflex
Copy link
Collaborator Author

@mikejolley think this can be merged? I can close and implement in my plugin, just figured it would be good to have in core

@kraftbj
Copy link
Contributor

kraftbj commented Aug 19, 2016

@chaselivingston tested, @mikejolley approved, @kraftbj merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants