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

Clean Up and Prettyify Location Metabox #57

Merged
merged 19 commits into from
Jan 24, 2018
Merged

Conversation

kingkool68
Copy link

@kingkool68 kingkool68 commented Jan 21, 2018

👋 David!

Before:
screen shot 2018-01-21 at 3 15 44 pm

After:
screen shot 2018-01-21 at 3 15 23 pm

  • Refactored and linted location.js
  • Cleaned up the form markup and grouped like-items together better
  • Added some pseudo-grid CSS classes to better align the fields to make them more visually pleasing
  • Replaced the icons with actual buttons that make clear what the action does
  • Add a confirmation before clearing location data (a destructive behavior)
  • Add a loading spinner while we wait to fetch geo data so things don't look broken

If the directory name changes, the scripts will break. Using `dirname(__FILE__)` will get the parent directory of the script currently being executed.
Now we don't need to spell out `jQuery` every time. We can just use the shorthand `$`.
Slight refactor to put initialization code after function definitons.
Handle this with an onclick method in jQuery instead.
…ctions.

If there is some sort of error then the default event action (follow a link etc.) will occur instead of being broken.
Otherwise the AJAX request will fail. Probably could remove the `console` statement all together.
I don't think we need to worry about the map zoom.
Just so we know the computer is doing something and not just sitting idle.
Use better markup and pseduo grid CSS classes.
@dshanske
Copy link
Owner

Can you fix the style issue that makes it fail phpcs validation?

@dshanske dshanske changed the base branch from master to pretty January 24, 2018 13:27
@dshanske
Copy link
Owner

Merging this into a branch to play with and do some other things before merging it into Master.

@dshanske dshanske merged commit 5ac0221 into dshanske:pretty Jan 24, 2018
@dshanske
Copy link
Owner

I'm going to do some additional work on this. I've decide I want to move the metabox code into its own file. It is getting way too long to be with everything else. It will be easier to maintain going forward if I separate it out.

@dshanske
Copy link
Owner

Continuing to work on this. Trying to get the improvements, but return to a view with less height.

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

Successfully merging this pull request may close these issues.

None yet

2 participants