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

Adds justification controls to Social Block #19386

Closed
wants to merge 2 commits into from

Conversation

getdave
Copy link
Contributor

@getdave getdave commented Jan 2, 2020

Screen Shot 2020-01-02 at 14 07 35

Description

Closes #18951.

Adds the ability to justify the horiztonal alignment of the Social Icons. This is achieved similarly to the Navigation Block.

As with that example, please note the distinction between "align" (which is really "float") and "justify" as terms.

I have noted that align (as opposed to justify) left/right is already available on this Block but it actually applies float left/right to the Block as a whole rather than moving the inner items.

Justification controls allow the user to manipulate the arrangement of the inner items whilst preserving (or manipulating separately) the positioning of the outer Block. Note, however, that this causes some ambiguity in the UI within the Editor when align and justify are applied together. It would be great if @jasmussen were able to advise on this as he was involved in the Nav Block and has the requisite context.

Questions

  • how can we resolve the layout within the editor when applying align and justification at the same time?
  • should we add align wide/full to this Block?
  • should we create a reusable "BlockJustificationToolbar" as per the BlockAlignmentToolbar and BlockVerticalAlignmentToolbar?

How has this been tested?

Manual testing.

  1. Add Social Block
  2. Add some items.
  3. Toggle the Justification.

Screenshots

Editor

Screen Capture on 2020-01-02 at 14-08-13

Frontend

Note: I've contained the Block within a Group Block with a background to show the boundaries of the justification:

Screen Shot 2020-01-02 at 14 40 57

Types of changes

New feature (non-breaking change which adds functionality)

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.
  • I've updated all React Native files affected by any refactorings/renamings in this PR. .

@getdave getdave added the [Block] Social Affects the Social Block - used to display Social Media accounts label Jan 2, 2020
@getdave getdave requested a review from mkaz as a code owner January 2, 2020 14:10
@getdave getdave self-assigned this Jan 2, 2020
@getdave getdave requested a review from jasmussen January 2, 2020 14:11
@youknowriad
Copy link
Contributor

In other blocks like "paragraph" we also have "align" and "text align". I wonder if "justify" is just another way of calling "text align".

There's also an issue about addiing a way to clear floats for all blocks which will create multiiple ways to achieve the same behavior.

@getdave
Copy link
Contributor Author

getdave commented Jan 2, 2020

I wonder if "justify" is just another way of calling "text align".

It is...sort of. But (IMHO) "justify" is more semantically correct. That said, if UX shows users understand "text align" to mean "control justification of content along the main axis" then who am I to argue.

I would say however, that text align feels very specific to the literal interpretation of "aligning some text". Whereas justify as a concept is to control position or space something along a line of content.

I also draw on the CSS Flexbox definitions which applies the "justify" nomenclature to denote spacing along the main axis:

The CSS justify-content property defines how the browser distributes space between and around content items along the main-axis of a flex container, and the inline axis of a grid container.

@getdave
Copy link
Contributor Author

getdave commented Jan 3, 2020

I'd value any input on the questions posed above under Questions.

@shaunandrews
Copy link
Contributor

I have noted that align (as opposed to justify) left/right is already available on this Block but it actually applies float left/right to the Block as a whole rather than moving the inner items.

Justification controls allow the user to manipulate the arrangement of the inner items whilst preserving (or manipulating separately) the positioning of the outer Block. Note, however, that this causes some ambiguity in the UI within the Editor when align and justify are applied together.

I struggle with this distinction. To me, the word "justify" is strongly associated with text. It feels like we're using justify because align is already in use. Maybe we should change the word align to "position", and then replace "justify" with "align".

image

I'm not sure we need to have a separate icon when aligning text vs. blocks (or any other elements) — the text align icons seem to work fine for both situations.

@obenland
Copy link
Member

obenland commented Jan 7, 2020

I like @shaunandrews‘ idea. Updating the verbiage in that way feels more natural to me.

@mtias
Copy link
Member

mtias commented Jan 7, 2020

I like simplifying to alignment with the text icons. We might want to support space-between in addition to center, maybe also space-around. How would we represent them?

@ZebulanStanphill
Copy link
Member

@shaunandrews I agree that "position" works a lot better for what we currently call "alignment" in the block toolbar. "position wide" makes a lot more sense than "align wide".

@mtias Perhaps those options could be represented with icons similar to this:
space-between-and-space-around-icon-mockups

@draganescu
Copy link
Contributor

should we create a reusable "BlockJustificationToolbar" as per the BlockAlignmentToolbar and BlockVerticalAlignmentToolbar?

With the addition of the buttons block this seems even more ... justified :D (sorry)

@getdave
Copy link
Contributor Author

getdave commented Jan 9, 2020

@draganescu You win the internet 😆


Thanks for everyone's comments. To summarise:

  • Rename to Positioning
  • Support more Flex properties

I'm not clear on the proposed route for icons. Are we going with what @shaunandrews posted above plus the additional icons (yet to be provided) for space-around/between?

@ZebulanStanphill
Copy link
Member

ZebulanStanphill commented Jan 9, 2020

I was confused and thought the "positioning" name was being suggested for the current alignleft/alignright/alignwide/etc. toolbar used by many blocks. I still think "justify" or "item alignment" would be a better term for the the toolbar being added in this PR.

@mtias
Copy link
Member

mtias commented Jan 9, 2020

Yes, I thought we were discussing using alignment for the distributions of elements within the container (left, center, space-between, etc) and position for how the container is displayed (wide, full-width, floated, etc).

@shaunandrews
Copy link
Contributor

Quick reference from Figma that offers similar options:

image

...and Sketch's variation:

image

Base automatically changed from master to trunk March 1, 2021 15:42
@jasmussen
Copy link
Contributor

We have justification controls for navigation, social links, and buttons now. I guess this PR can be closed? Thanks for all the work!

@mkaz
Copy link
Member

mkaz commented Mar 8, 2021

Closed in #28980

@mkaz mkaz closed this Mar 8, 2021
@sirreal sirreal deleted the add/justification-support-to-social-block branch September 25, 2024 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Social Affects the Social Block - used to display Social Media accounts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Social Block: Update alignment controls
9 participants