Skip to content
This repository was archived by the owner on Dec 2, 2020. It is now read-only.

Try: Adding "Portfolio List" block pattern + modifying "wide" separator style #299

Merged
merged 7 commits into from
Oct 12, 2020

Conversation

kjellr
Copy link
Collaborator

@kjellr kjellr commented Oct 6, 2020

Would address part of #50.

This is a first pass at achieving the "Portfolio List" block pattern, using just what's available today in Gutenberg.

Original Mockup Current State
93506798-c2a74800-f8ea-11ea-9994-8a44ba402e33-1 Screen Shot 2020-10-06 at 1 54 16 PM

A few caveats:

  • Using default Gutenberg tools, It's not possible today to easily inline an image next to the text. There is an inline image option, but it's not suitable for use in a Block Pattern because it does not include any way to replace the image. Instead, I've placed the images at the right for now.
  • I've modified the "Wide" separator style so that it includes the thicker stroke. I'm unable to make it wide-width for now though, because of the structure of the block in the editor (wide alignment styles would need to be applied to the parent of the separator block, and CSS isn't good about that). We'll need to wait for Add Align support to Separator block gutenberg#25147. In the meantime, this pattern is the default width instead of the wide width. 😕
  • While we work through this, I used only images that we've added to the repo already.
  • Mobile views of this look ok, but not great.

Screenshots

Editor

Screen Shot 2020-10-06 at 1 41 47 PM

Desktop Mobile
Screen Shot 2020-10-06 at 1 40 27 PM Screen Shot 2020-10-06 at 1 41 03 PM

@kjellr kjellr added the [Component] Block patterns Issues related to block patterns shipped with theme label Oct 6, 2020
@kjellr kjellr requested a review from melchoyce October 6, 2020 17:55
@kjellr kjellr self-assigned this Oct 6, 2020
@melchoyce
Copy link
Contributor

Man, we are kind of striking out with these patterns.

The separators + the link underlines make this feel really busy. What if the top and bottom separators were 3px, but the in-between ones were 1px instead?

image

It's not perfect, but it helps. Hopefully we'll be able to do try wide-width separators soon as well, because I think that would also help.

Since we can't use inline images, it might also help to float the images all the way to the right, and then cap all their heights at 67px:

image

Since they have different aspect ratios, this makes for a nice ragged edge.

@kjellr
Copy link
Collaborator Author

kjellr commented Oct 6, 2020

What if the top and bottom separators were 3px, but the in-between ones were 1px instead?

Good suggestion — I can try that out tomorrow.

it might also help to float the images all the way to the right

I tried that initially actually, but changed them back because it led to this on mobile. 😕

95241300-8d9c6000-07db-11eb-9259-6d4df6874275

@melchoyce
Copy link
Contributor

Can we wrap in a group block with a custom class, and tweak the mobile positioning?

@kjellr
Copy link
Collaborator Author

kjellr commented Oct 7, 2020

We can, but I'm not sure that we should. Using custom classnames in these seems like a hack to me. For simplicity's sake, I consider patterns to be something that the user should be able to just create themselves without having to open the "Advanced" panel and type in a custom classname. I could be being too conservative about them though.

@kjellr
Copy link
Collaborator Author

kjellr commented Oct 7, 2020

The separators + the link underlines make this feel really busy. What if the top and bottom separators were 3px, but the in-between ones were 1px instead?

I pushed an update to take care of this. In order to make that happen, I made the default separator 100% wide and 1px tall. That's more or less in line with the comps here anyway.

Screen Shot 2020-10-07 at 9 45 19 AM

it might also help to float the images all the way to the right, and then cap all their heights at 67px:

Mobile issues aside, I tried this too. But it highlighted a couple other issues.

  • The floated images mess with the equal spacing. 😕
  • Also, it seems like our alignleft/alignright styles are imposing some sort of (minimum?) enforced width, so I can't get that ragged edge working.

Screen Shot 2020-10-07 at 9 50 15 AM

@melchoyce
Copy link
Contributor

Weird, the screenshot from above is from my editor, so recent updates must have changed things. Would #295 by @aristath fix the width issue?

@kjellr
Copy link
Collaborator Author

kjellr commented Oct 7, 2020

I got the right alignment + rag sorted out. I had to explicitly set both the width and height of each image.

@melchoyce
Copy link
Contributor

Do we want to merge this and make a new issue to handle the responsive styles?

@kjellr
Copy link
Collaborator Author

kjellr commented Oct 7, 2020

I think that's reasonable, as long as folks are cool with the changes to the separator block included here. Here's a quick reference:

Before:

Screen Shot 2020-10-07 at 1 24 35 PM

After:

Screen Shot 2020-10-07 at 1 25 01 PM

@melchoyce
Copy link
Contributor

Good with me, let's just keep an eye on WordPress/gutenberg#25147 and maybe revisit 👍

@kjellr kjellr requested review from aristath and carolinan October 7, 2020 18:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
[Component] Block patterns Issues related to block patterns shipped with theme
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants