Skip to content
This repository has been archived by the owner on Oct 16, 2024. It is now read-only.

Toggle the inserter option on by default #87

Closed
wants to merge 4 commits into from

Conversation

mike-day
Copy link
Contributor

@mike-day mike-day commented Mar 3, 2023

Currently on #78, the inserter toggle is toggled "off" when a new pattern is first opened in the editor (but before the post is published — i.e., before you actually save the pattern by hitting Create Pattern).

But, if you leave the toggle "off", the value is automatically toggled "on" when the post is published.

To recreate:

  1. Stay on the branch for On creating a new pattern, don't write to the pattern PHP file until you publish #78
  2. Create a new pattern
  3. Note the inserter toggle state (should be "off") in the Post Types panel
  4. Go ahead and publish the post
  5. Note that the inserter toggle value is now "on"

This PR is just a suggestion for one way to get around this very minor issue. The toggle state will initially show as "on" (which I think is preferred), but you are free to toggle it "off" and have that value stay the same upon actually publishing the pattern.

How to test

  1. Checkout this branch
  2. Create a new pattern
  3. Note the inserter toggle state (should be "on")
  4. Toggle the inserter to "off"
  5. Publish the post
  6. Note that the inserter toggle value is still "off"

The solution offered here is somewhat imperative — there might be a better way to do this!

@johnstonphilip
Copy link
Contributor

Working here!

@@ -1 +1,5 @@
export type { BaseSidebarProps as ToggleTypes } from '../SidebarPanels/types';

export type AdditionalToggleTypes = {
Copy link
Contributor

@johnstonphilip johnstonphilip Mar 3, 2023

Choose a reason for hiding this comment

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

It's not obvious to me why some are considered "Additional" and others are not. Is it possible to add a comment to clarify that here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I closed this PR, so these additional props are not being merged. But, to circle back — the base types are intended to be common between toggle components, while additional might only be used for one component and picked like this in the component:

export default function SomeComponent( {
  someProp,
  anotherProp,
}: BaseProps & Pick< AdditionalProps, 'extraProp' > ) {
  ...
}

It's a really useful pattern for SidebarPanels, but it might honestly be over-abstraction here!

Copy link
Contributor

@kienstra kienstra left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this.

Does this fix it in your local?

(setting the default for 'toggle' via register_post_meta())

diff --git a/wp-modules/editor/editor.php b/wp-modules/editor/editor.php
index abcdd7cd..52b6bd9a 100644
--- a/wp-modules/editor/editor.php
+++ b/wp-modules/editor/editor.php
@@ -79,6 +79,7 @@ function register_pattern_post_type() {
                        'show_in_rest' => true,
                        'single'       => true,
                        'type'         => 'boolean',
+                       'default'      => true,
                )
        );
 

@mike-day
Copy link
Contributor Author

mike-day commented Mar 3, 2023

Thanks for fixing this.

Does this fix it in your local?

(setting the default for 'toggle' via register_post_meta())

diff --git a/wp-modules/editor/editor.php b/wp-modules/editor/editor.php
index abcdd7cd..52b6bd9a 100644
--- a/wp-modules/editor/editor.php
+++ b/wp-modules/editor/editor.php
@@ -79,6 +79,7 @@ function register_pattern_post_type() {
                        'show_in_rest' => true,
                        'single'       => true,
                        'type'         => 'boolean',
+                       'default'      => true,
                )
        );
 

Great idea! It does correct the initial state of the toggle, but it flips to "off" automatically when I publish the pattern.

@kienstra
Copy link
Contributor

kienstra commented Mar 3, 2023

Ahhh, OK.

@kienstra
Copy link
Contributor

kienstra commented Mar 3, 2023

Hi @mike-day,
Is this the expected behavior? It's working locally with the snippet above, though I might not be understanding what's expected.

  1. checkout On creating a new pattern, don't write to the pattern PHP file until you publish #78
  2. Apply this snippet
  3. Go to /wp-admin/post-new.php?post_type=pm_pattern
  4. Only add some content, don't change anything else
  5. Click 'Create Pattern'
  6. Expected: The 'Display in inserter' toggle is still toggled on
  7. Reload browser
  8. Expected and actual: 'Display in inserter' is still toggled on:

Screenshot 2023-03-03 at 11 14 51 AM

@mike-day
Copy link
Contributor Author

mike-day commented Mar 3, 2023

@kienstra it does work — I had something in my local that was throwing the result off.

I am going to close this suggestion out since the issue is fixable with 1 new line (as opposed to my overcomplicated solution)!

@mike-day mike-day closed this Mar 3, 2023
@kienstra
Copy link
Contributor

kienstra commented Mar 3, 2023

Thanks, Michael!

@kienstra
Copy link
Contributor

kienstra commented Mar 3, 2023

Overcomplicated solutions happen. I probably do them more often than not.

And you were fixing a bug in #78, so I really appreciate you working on it.

@mike-day mike-day deleted the suggestion/update/new-post branch March 3, 2023 17:44
@mike-day
Copy link
Contributor Author

mike-day commented Mar 3, 2023

Of course! I don't mind being wrong, especially if it means learning something new in process 😄

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants