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

Add notice for block patterns new location. #41963

Merged
merged 5 commits into from
May 8, 2020

Conversation

Addison-Stavlo
Copy link
Contributor

@Addison-Stavlo Addison-Stavlo commented May 7, 2020

Changes proposed in this Pull Request

As block patterns were recently moved, there have been a number of support requests from users that have not been able to find them.

This reintroduces the block patterns sidebar plugin to notify users of the change. This is intended to be temporary and removed in the coming weeks after users have had a chance to adapt to this change.

Testing instructions

  • Build / Sync FSE plugin on this branch.
  • Go to the editor, ensure the block patterns Icon is in the top-right toolbar and in the dropdown menu.
  • Verify clicking the icon shows a message that block patterns have moved.
  • Verify clicking the example icon opens the block patterns inserter.

Screen Shot 2020-05-07 at 7 36 19 PM


Screen Shot 2020-05-07 at 7 36 37 PM


Screen Shot 2020-05-07 at 8 10 50 PM


Fixes #41930

@Addison-Stavlo Addison-Stavlo requested review from a team as code owners May 7, 2020 23:41
@matticbot
Copy link
Contributor

@Addison-Stavlo Addison-Stavlo changed the title Add/block pattern moved popover Add notice for block patterns new location. May 7, 2020
@Addison-Stavlo Addison-Stavlo self-assigned this May 7, 2020
@matticbot
Copy link
Contributor

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

@matticbot
Copy link
Contributor

Caution: This PR affects files in the FSE Plugin on WordPress.com
Please ensure your changes work on WordPress.com before merging.

D43148-code has been created so you can easily test it on your sandbox. See this FieldGuide page about developing in the FSE Plugin for more info: PCYsg-ly5-p2

@Addison-Stavlo Addison-Stavlo requested a review from sirreal May 8, 2020 00:09
);
};

registerPlugin( 'block-patterns-moved', { render: BlockPatternsMoved } );
Copy link
Member

@simison simison May 8, 2020

Choose a reason for hiding this comment

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

Both plus and layout icons were added in Gutenberg v7.6 (WordPress/gutenberg#20234, WordPress/gutenberg#20181) and patterns were added in v7.7. It's not exact but we could just not register this if those icons are undefined? Those users never had patterns to beging with. I think we never shipped v7.6 and went straight to v7.7 from v7.3, but on Atomic there are always users across all versions.

Will avoid fatals in older Gutenbergs on Atomic anyway.

Copy link
Member

Choose a reason for hiding this comment

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

icons is unique, it's bundled rather than external.

Copy link
Member

Choose a reason for hiding this comment

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

Ahh right, forgot about that. So then that doesn't matter, it's not a biggie to ship this to older versions. Will just increase awareness. :-)

Copy link
Member

@simison simison left a comment

Choose a reason for hiding this comment

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

I'll leave testing for someone else but otherwise looks good. 👍

I hoped we didn't need sidebar and a simple popover or tooltip would be enough, but this is ok too.

Nice that you added bonus of opening the patterns. :-) There might be some data-dispatch way opening it also but considering this will be there for 1 or 2 versions, this seems ok.

className="common__block-patterns-moved"
>
<PanelBody>
<h2>{ __( 'Block Patterns have moved!' ) }</h2>
Copy link
Member

Choose a reason for hiding this comment

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

I think we might need full-site-editing textdomain here.

See #41966 (comment)

@mtias
Copy link
Member

mtias commented May 8, 2020

Nice! I agree a popover should be enough, and not sure we should add the "Have Moved..." to the name / plugin.

@sirreal sirreal self-assigned this May 8, 2020
@sirreal
Copy link
Member

sirreal commented May 8, 2020

There doesn't seem to be a way to render a menu button in the top menu without adding a sidebar. We can render a popover out of the sidebar menu when it's opened.

I've pushed some copy tweaks and added a text domain.

@Addison-Stavlo
Copy link
Contributor Author

Addison-Stavlo commented May 8, 2020

There doesn't seem to be a way to render a menu button in the top menu without adding a sidebar.

The sidebar plugin seemed the least-hacky way of doing things. We could possibly create a button with a popover and append it? but that would require setting a timeout interval to wait for the selectors we need to append it to to appear. Beyond that, we would also need to find a way to append it to the menu as this was another place where users could previously find their block patterns. Registering a plugin seemed like the simple way to get this set up, but I agree the sidebar is a bit unnecessary.

We can render a popover out of the sidebar menu when it's opened.

Does a popover add value at that point?

Nice that you added bonus of opening the patterns. :-) There might be some data-dispatch way opening it also but considering this will be there for 1 or 2 versions, this seems ok.

The data dispatch would be nice. With my recent experience working on some core things, it seems there is a bit of a reluctance to inflate in the data store with things that can be handled by local state. Given that observation, I assumed there wouldn't be a way to dispatch opening the 'patterns' tab at the very least.

@sirreal
Copy link
Member

sirreal commented May 8, 2020

Does a popover add value at that point?

Probably not, it does point to the component that needs to be clicked, but it's a lot of UI jumping out at once, probably too much.

There might be some data-dispatch

I looked and the inserter panel opening was local component state. It's not exposed via a data store.

@simison
Copy link
Member

simison commented May 8, 2020

I'm fine with sidebar — it's not as pretty as popover but does the job.

It's not worth spending too much time on this — let's get it in and get those lost customers use the patterns again.

I agree calling the sidebar just "Patterns" for stylistic reasons.

I'd say we can keep this out for 2 or 3 weeks. Randomly made that up and totes open to other timings. ;-)

@Addison-Stavlo
Copy link
Contributor Author

I'd say we can keep this out for 2 or 3 weeks.

That timing makes sense to me. How long did block patterns live in this sidebar? a month-ish? removing it after another ~2 releases seems to make sense.

@simison
Copy link
Member

simison commented May 8, 2020

How long did block patterns live in this sidebar?

Since March 25, 2020 (v7.7.1).

Moved to a new spot in May 5, 2020 (v8.0.0).

(via PCYsg-m14-p2)

Can you add a note in the Fieldguide page so that alongside some version bump next gardeners know to remove this? Thanks!

@Addison-Stavlo Addison-Stavlo merged commit c3eb020 into master May 8, 2020
@Addison-Stavlo Addison-Stavlo deleted the add/block-pattern-moved-popover branch May 8, 2020 20:00
@scinos scinos mentioned this pull request May 11, 2020
ockham added a commit that referenced this pull request May 25, 2020
This PR essentially reverts #41963, which introduced the block patterns sidebar to point people to their new location. That sidebar was always meant to be temporary -- it's been in for about 2 weeks, so we're going to remove it in the next FSE plugin version.
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.

Patterns: guide people to new patterns UI
5 participants