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

Markdown block: move support link away from description #28416

Closed
wants to merge 2 commits into from

Conversation

simison
Copy link
Member

@simison simison commented Nov 9, 2018

According to Gutenberg handbook, descriptions should be kept simple.

Even though our way of adding the support link directly in description works now (at least for desktop), we should keep description semantically free from custom UI. I'd imagine description popping up in different part sof the UI in future, especially for mobile clients.

Basically, this isn't visually as nice but semantically more correct.

Before

screenshot 2018-11-09 at 09 49 20

After

screenshot 2018-11-09 at 09 38 05

Changes proposed in this Pull Request

  • Move support link from description to block inspector.

Testing instructions

  • Spin up Jetpack site gutenpack-jn, connect jetpack and enable markdown module
  • Add markdown block to a Gutenberg post
  • Note support link in the sidebar when block is selected

@simison simison added [Pri] Low [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Status] Needs Design Review Add this when you'd like to get a review / feedback from the Design team on your PR [Goal] Gutenberg Working towards full integration with Gutenberg labels Nov 9, 2018
@matticbot
Copy link
Contributor

Copy link
Member

@tyxla tyxla left a comment

Choose a reason for hiding this comment

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

This is a great little change 👍I claim this also looks better than before. Thanks!

Tests well and looks good 🚢

@tyxla tyxla removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Nov 9, 2018
@mapk
Copy link
Contributor

mapk commented Nov 9, 2018

This change is executed nicely, but I'm still not a big fan. While the Gutenberg docs recommend against long descriptions and branding in this section, I don't think having a link there conflicts with that.

The description area "describes" the block and if the user doesn't know much about that, the link provides more description elsewhere. They seem to go well together.

With the link now in its own section, I begin to feel like its lacking validation for this. My vote is to keep it the way it was, but I'm open to the change.

@simison
Copy link
Member Author

simison commented Nov 10, 2018

We could adjust CSS to make it look again like it used to, while keeping the "description" field free from any HTML.
@mapk Would that work?

In fact I think I'll move this to be shared component so I can re-use it with Simple Payments and other block.

@MichaelArestad
Copy link
Contributor

I think it was better before. Adding it as it's own metabox seems to feel fairly disconnected.

@simison
Copy link
Member Author

simison commented Nov 13, 2018

Adding it as it's own metabox seems to feel fairly disconnected.

You're totally right.

How about with little CSS tweak:

image

Best of both worlds — semantically correct in code and visually as it was before. :-)

Technically it's still its own section and I've just removed the border and adjusted margin.

Copy link
Member

@tyxla tyxla left a comment

Choose a reason for hiding this comment

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

My opinion is that if we are to hack our way to make it look as part of the description, actually keeping it a part of the description makes more sense.

// Sidebar
.components-panel .jetpack-markdown__support {
border-top: 0;
padding-left: 54px;
Copy link
Member

Choose a reason for hiding this comment

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

Now there is where it feels off 😕

Copy link
Member Author

Choose a reason for hiding this comment

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

Hehehe agreed with this too. :-) Non-ideal solutions all around!

@simison
Copy link
Member Author

simison commented Nov 13, 2018

Closing; I think this was enough of an exploration into what we could have and it's a pity there isn't an ideal solution.

Let's hope we'll get official declarative support for documentation links later on:
WordPress/gutenberg#11330

Let's also make sure that all our blocks add support links in exact same way to stay consistent.

@simison simison closed this Nov 13, 2018
@simison simison deleted the update/markdown-block-support-link branch November 13, 2018 08:35
@simison simison removed the [Status] Needs Design Review Add this when you'd like to get a review / feedback from the Design team on your PR label Nov 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Goal] Gutenberg Working towards full integration with Gutenberg [Pri] Low
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants