-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Conversation
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. |
Caution: This PR affects files in the FSE Plugin on WordPress.com 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 |
); | ||
}; | ||
|
||
registerPlugin( 'block-patterns-moved', { render: BlockPatternsMoved } ); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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. :-)
There was a problem hiding this 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> |
There was a problem hiding this comment.
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)
Nice! I agree a popover should be enough, and not sure we should add the "Have Moved..." to the name / plugin. |
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. |
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.
Does a popover add value at that point?
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. |
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.
I looked and the inserter panel opening was local component state. It's not exposed via a data store. |
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. ;-) |
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. |
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! |
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.
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
Fixes #41930