Optional modal for editor fields #93

kienstra · 2021-08-31T15:32:46Z

Changes

On clicking the <ServerSideRender>, this will show the block form in a <Modal>.

Feature Walkthrough (updated September 24)

composer i && npm i && npm run dev
/wp-admin > Custom Blocks > Add New
Check 'Open block fields in a modal…' (it's unchecked by default):
Add a field
Publish the block
Create a new post
Add the block you created to that post
Click the block:
Expected: A modal opens:

On clicking the <ServerSideRender>, this will show the block form in a <Modal>.

There was a conflict in edit.js.

No need to wrap that in ()

To save in the block properties.

Testing that it's not open initially but it opens on clicking.

…electors Might have to revert this, but it simplifies it.

Also, add engine-strict = true to .npmrc

kienstra · 2021-09-01T21:39:31Z

Hi @RobStino,
What do you think about how the block form looks in the modal?

It's basically the same styling as in the block editor.

This still isn't ideal, even if it worked. It's opaque why this needs to click escape.

This reverts commit f026292.

Thanks to this for the solution: WordPress/gutenberg#12830 (comment)

kienstra · 2021-09-06T21:11:38Z

.gitignore

-
-# ack
-.ackrc
-


Not related to this PR, but some of these ignores aren't needed.

kienstra · 2021-09-06T21:12:21Z

composer.json

+            "phpcs"
+        ],
+        "lint-fix": [
+            "phpcbf"


Might as well run phpcbf and phpcs from within Composer, instead of running them like ./vendor/bin/phpcbf

Also, correct some JSDoc headings. Those were from copypasta.

kienstra · 2021-09-06T21:22:56Z

js/src/block-editor/controls/image.js

 									>
 										{ __( 'Upload', 'genesis-custom-blocks' ) }
 									</FormFileUpload>
 									<MediaUploadCheck>
 										<MediaUpload
-											gallery={ false }
-											multiple={ false }


These are default props, so no need to pass them

This removed the Inspector styling, and it wasn't showing.

Remove an extra class, as the first class is for the same element.

Just slightly modified from Phil's idea. This should make it much clearer what the checkbox does.

Before, it was using block.label, which didn't exist. Also, import the Block definition to prevent this.

kienstra · 2021-09-07T18:56:29Z

Thanks to Phil's idea, this now shows the block's name as the title of the <Modal>:

kienstra · 2021-09-07T18:58:01Z

Thanks to Phil for his idea of new <label> text:

And add a Jest test that it converts UpperCase to lowercase.

johnstonphilip · 2021-09-07T19:35:35Z

Working well for me here!

johnstonphilip · 2021-09-07T19:36:12Z

I wish that when I changed a field in the modal that it would reflect in the background imediately instead of showing a reloading icon spinner for a second. But that's not really a blocker.

kienstra · 2021-09-07T19:47:28Z

I wish that when I changed a field in the modal that it would reflect in the background imediately instead of showing a reloading icon spinner for a second.

Yeah, that's poor UX. Maybe the post you shared about rendering the template in the editor could help.

It'd probably have dangerouslySetInnerHTML.

It will fail if the query fails, anyway.

kienstra · 2021-09-07T19:49:23Z

@johnstonphilip, thanks a lot for your great ideas and for testing this!

mikemcalister · 2021-09-07T19:53:38Z

Really like this improvement, Ryan! Worked well in my testing.

kienstra · 2021-09-07T19:54:04Z

@mikemcalister,
Thanks a lot! Really appreciate your testing and expertise.

kienstra added 17 commits August 23, 2021 16:43

Initial POC of the editor modal

d7dbfe5

On clicking the <ServerSideRender>, this will show the block form in a <Modal>.

Merge in develop, resolve conflict

c731c4a

There was a conflict in edit.js.

In composer.json, add a lint and lint-fix script

bc9961e

Access isSelected from blockProps

2a1ba28

Remove needless brackets and return statements

ef4ff50

Simplify the <EditorForm> rendering

446f7c4

No need to wrap that in ()

Do npm run packages-update for WP packages

4a76b10

Update more packages to work on getting Jest tests to pass

066dfd0

Revert changes to package-lock.json and package.json

8967ec8

Add a checkbox to display in the modal

e84633f

To save in the block properties.

Remove 2 extra spaces in composer.json

11132c5

Inline the handleChange() function in the onChange prop

02704ee

Remove some needless patterns from .gitignore

52239d8

Add Jest tests for the modal

254add0

Testing that it's not open initially but it opens on clicking.

Add an ESLint rule for the textdomain in i18n functions

ad8a33d

Change the styling to only look for .block-form, not other wrapping s…

25e8c7a

…electors Might have to revert this, but it simplifies it.

Remove default values from <MediaUpload> and <FormFileUpload>

6e9f837

Also, add engine-strict = true to .npmrc

kienstra added 3 commits September 3, 2021 12:11

Possible solution to bug, though it takes too long

f026292

This still isn't ideal, even if it worked. It's opaque why this needs to click escape.

Revert "Possible solution to bug, though it takes too long"

259c00f

This reverts commit f026292.

Fix an issue with a <MediaUpload> inside the <Modal>

6d63f9d

Thanks to this for the solution: WordPress/gutenberg#12830 (comment)

kienstra commented Sep 6, 2021

View reviewed changes

.gitignore

# ack

.ackrc

Copy link

Contributor Author

kienstra Sep 6, 2021

Choose a reason for hiding this comment

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

Not related to this PR, but some of these ignores aren't needed.

kienstra commented Sep 6, 2021

View reviewed changes

Add an assertion that displayModal is false

142fb4b

Also, correct some JSDoc headings. Those were from copypasta.

kienstra commented Sep 6, 2021

View reviewed changes

kienstra marked this pull request as ready for review September 6, 2021 21:23

kienstra added 3 commits September 6, 2021 16:48

Fix the styling in the Inspector

e4b2fe1

This removed the Inspector styling, and it wasn't showing.

Fix an issue where the Color control had missing styling

3244538

Remove an extra class, as the first class is for the same element.

Add Inner Blocks and File field to README.md

2a64981

kienstra requested a review from RobStino September 7, 2021 02:53

kienstra requested review from johnstonphilip, dreamwhisper, marksabbath, mikemcalister and BMO-tech September 7, 2021 02:53

kienstra added 2 commits September 7, 2021 13:54

Use Phil's idea for <label> text for the checkbox

16faf33

Just slightly modified from Phil's idea. This should make it much clearer what the checkbox does.

Add a correct title for the <Modal>

71b9aba

Before, it was using block.label, which didn't exist. Also, import the Block definition to prevent this.

kienstra added 2 commits September 7, 2021 14:23

Ensure that the slug is lower case

ed3c4f1

And add a Jest test that it converts UpperCase to lowercase.

Allow multiple block titles, as it's now in the <Modal> header

41e347d

johnstonphilip approved these changes Sep 7, 2021

View reviewed changes

Don't call toBeInTheDocument() in test

d07a86b

It will fail if the query fails, anyway.

mikemcalister approved these changes Sep 7, 2021

View reviewed changes

kienstra merged commit 3b74d78 into develop Sep 10, 2021

kienstra deleted the add/editor-modal branch September 10, 2021 15:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optional modal for editor fields #93

Optional modal for editor fields #93

kienstra commented Aug 31, 2021 •

edited

Loading

kienstra commented Sep 1, 2021

kienstra Sep 6, 2021

kienstra Sep 6, 2021

kienstra Sep 6, 2021

kienstra commented Sep 7, 2021

kienstra commented Sep 7, 2021

johnstonphilip commented Sep 7, 2021

johnstonphilip commented Sep 7, 2021

kienstra commented Sep 7, 2021

kienstra commented Sep 7, 2021

mikemcalister commented Sep 7, 2021

kienstra commented Sep 7, 2021

Optional modal for editor fields #93

Optional modal for editor fields #93

Conversation

kienstra commented Aug 31, 2021 • edited Loading

Changes

Feature Walkthrough (updated September 24)

kienstra commented Sep 1, 2021

kienstra Sep 6, 2021

Choose a reason for hiding this comment

kienstra Sep 6, 2021

Choose a reason for hiding this comment

kienstra Sep 6, 2021

Choose a reason for hiding this comment

kienstra commented Sep 7, 2021

kienstra commented Sep 7, 2021

johnstonphilip commented Sep 7, 2021

johnstonphilip commented Sep 7, 2021

kienstra commented Sep 7, 2021

kienstra commented Sep 7, 2021

mikemcalister commented Sep 7, 2021

kienstra commented Sep 7, 2021

kienstra commented Aug 31, 2021 •

edited

Loading