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

WIP: Add/social block movers #17701

Closed
wants to merge 15 commits into from
Closed

WIP: Add/social block movers #17701

wants to merge 15 commits into from

Conversation

draganescu
Copy link
Contributor

Description

Depends on #16615

Implements movers for the Social Icons Block.

How has this been tested?

Enabled the option and tested locally

Screenshots

social-icons

Types of changes

  • The InnerBlocks component get a new set of options.
  • Removed the mover hiding CSS

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.

Copy link
Member

@jorgefilipecosta jorgefilipecosta left a comment

Choose a reason for hiding this comment

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

The movers seem to work well. This PR applies with success the API being implemented in #16615, I left some comments on the API on the other PR.
For reference this is how the movers look on social links:
image

@jorgefilipecosta jorgefilipecosta added the Needs Design Feedback Needs general design feedback. label Oct 2, 2019
@draganescu draganescu changed the base branch from master to update/horizontal-block-movers October 3, 2019 09:25
@draganescu draganescu changed the base branch from update/horizontal-block-movers to master October 3, 2019 09:25
@jasmussen
Copy link
Contributor

Ah, nice! Let's rebase when #16615 gets merged. Based on the GIF, this one is close, but needs a little love with regards to how the horizontal mover looks. Based on the GIF it looks like it's not quite as tall as it needs to be, which I fixed in 16615, and that it needs a little less space to the left. Both things I can fix easily, but I'd like to wait for the rebase to make sure I'm not duplicating work. Very cool.

@draganescu
Copy link
Contributor Author

I am going to close this branch and do it again once the horizontal mover PR is in master.

@draganescu draganescu closed this Oct 20, 2019
@youknowriad youknowriad deleted the add/social-block-movers branch October 21, 2019 08:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Design Feedback Needs general design feedback.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants