Introduce a ResizableBox component #10347

chrisvanpatten · 2018-10-04T19:43:21Z

This is a follow-up to #10331 and fixes #10343.

This PR…

introduces a ResizableBox component which wraps the re-resizable package to remove some code duplication
introduces global/shared classes for resize handles
removes the mixins introduced in Try: Larger drag handles #10331, and thus the code duplication (which had existed for a while before that)
updates core/image and core/spacer to use this new component

~~I plan to flesh out the docs, but want to make sure I'm on the right track with this first!~~ Still want to know if I'm on track, but I added additional docs anyway :)

Testing

Insert image and spacer blocks
Verify there are no visual or functional regressions at various alignments and configurations

tofumatt · 2018-10-04T20:39:30Z

Thanks for jumping on the patch for this! 😄

I'l try to take a look this week, but I might have to wait a bit until after we clear out the 4.0 milestone. 😅

chrisvanpatten · 2018-10-05T12:22:24Z

@tofumatt Also should note that if you want to take this off your plate, feel free to unassign yourself from the review! I requested you because you had reviewed #10331 but definitely don't feel obligated to review this too :) I know you have a lot going on!

tofumatt

I dig it! I think we need to add to the components changelog to mention the new component we've introduced. 🤔

After that I think this is good! Thanks for doing this! ❤️

tofumatt · 2018-10-05T13:58:47Z

packages/components/src/resizable-box/README.md

+
+## Usage
+
+Most options are passed directly through to [re-resizable](https://github.com/bokuweb/re-resizable) so you may wish to refer to their documentation. Note that we set the `handleClasses` and `handleStyles` to enable the Gutenberg-specific styles.


I'd argue that we don't so much set them as unset them; it might be worth clarifying exactly what we do here for clarity.

Fair enough, I can see how it would be slightly confusing… we're setting the prop but unsetting the values. I'll update this :)

chrisvanpatten · 2018-10-05T14:34:50Z

@tofumatt Clarified the docs and added to the changelog. Might need a check from @gziolo to be sure I updated the changelog correctly (although I do believe it's right; I checked the file history to see how it had been handled previously).

tofumatt · 2018-10-05T14:44:44Z

packages/components/CHANGELOG.md

@@ -1,3 +1,9 @@
+## 5.0.0 (Unreleased)


Not sure I'd count this as a 5.0.0 feature; it's more like 4.1.0. It isn't a breaking change and it's just an addition.

Fair point :)

tofumatt · 2018-10-05T19:00:04Z

I think the test failures are unrelated, see: #10364.

Once that's merged you should probably rebase and then flag me for another review... but I think this is all good 👍

chrisvanpatten · 2018-10-05T19:27:52Z

@tofumatt looks like Travis is still failing for the Undo and Split/Merge tests, which is likely unrelated.

tofumatt · 2018-10-05T19:28:46Z

Yup, that one is an "expected" intermittent failure 😅 (restarted)

tofumatt · 2018-10-05T21:33:27Z

It keeps failing, maybe something else is up? 😢

chrisvanpatten · 2018-10-05T22:46:49Z

@tofumatt I’ll try the tests locally. It’s strange it only happens in the one environment in Travis…

chrisvanpatten · 2018-10-06T14:21:24Z

I got the local test environment set up and I can't get the undo test working there (split/merge seems good now)… but I also can't get the undo test working in master so it seems like just a sporadic/unpredictable/unrelated failure.

tofumatt · 2018-10-06T16:43:04Z

Ten four, I've kicked the test server again and hopefully it'll work done 🤷‍♂️

chrisvanpatten · 2018-10-06T17:28:10Z

Strangely I can't "pass" the undo test in master (or this branch) even when I manually follow the instructions (typing in three lines and hitting undo six times). It actually takes me 7 or 8 undos before I'm back to a disabled undo state.

I'm wondering if this might actually be a bug in master right now? I can't reproduce it in 3.9.

chrisvanpatten · 2018-10-06T17:28:37Z

lol okay just as I type that the test finally passes ¯_(ツ)_/¯

@tofumatt I think we're finally clear on this… phew…

(Also should it still be held until 4.1? Will there still be a 4.1 at this point…?)

tofumatt · 2018-10-06T22:17:10Z

Miguel and I were saying that actually replicating the test case manually indicates it might be a bit off. I think that test case could use some deeper examination… - Matt (Sent from mobile)

…

On 6 Oct 2018, at 13:28, Chris Van Patten ***@***.***> wrote: lol okay just as I type that the test finally passes ¯_(ツ)_/¯ @tofumatt I think we're finally clear on this… phew… — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

tofumatt · 2018-10-06T22:52:39Z

In general I’m for merging as much stuff as possible and this is pretty minimal. I’ll have a look tomorrow, been a bit distracted by travel today.

There will be a 4.1 by the way! In general Gutenberg development (including releases) will continue on GitHub, so while I’m not exactly sure how things will work after WordPress 5.0, I think the general idea will be that certain releases of WordPress will pull in a certain release of Gutenberg, if that makes sense.

chrisvanpatten · 2018-10-07T00:00:30Z

@tofumatt totally fair! Travel ~~can’t~~ can knock the wind out of you.

I’m going to try a git ~~blame~~ bisect to see if I can figure out when the undo behaviour changed and if I can figure it out I’ll open a new issue.

chrisvanpatten · 2018-10-07T02:00:07Z

@tofumatt just opened #10380 which tracks the undo issue. It looks related to the rich text state structure changes, which makes sense.

tofumatt · 2018-10-07T05:39:23Z

Thanks for tracking that one down! - Matt (Sent from mobile)

…

On 6 Oct 2018, at 22:00, Chris Van Patten ***@***.***> wrote: @tofumatt just opened #10380 which tracks the undo issue. It looks related to the rich text state structure changes, which makes sense. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

tofumatt

I dig it 👍

aduth · 2018-10-10T14:22:43Z

This introduced changes when running npm install on master because the package-lock.json file wasn't updated.

aduth · 2018-10-10T14:24:17Z

#10234

chrisvanpatten · 2018-10-10T14:24:43Z

Ughhh I'm really sorry @aduth 😭 I should have caught that…

aduth · 2019-05-02T19:14:23Z

packages/components/src/resizable-box/style.scss

+	display: none;
+
+	// Show the resize handle when selected.
+	.components-resizable-box__container.is-selected & {


Where do we document how a resizable box becomes "selected" to unhide itself? The current documentation for ResizableBox has no indication of this default hidden state. It was quite the journey to find my way here, as I've found it quite difficult to use the component with the documentation as written.

The behavior here was inherited from the original, non-componentized usages of ResizableBox; you can see it in previous PRs as well such as here.

There are probably other use-cases that would provide more valuable context, and could certainly be referenced and improved in the documentation.

While I'd agree it often needs contextualized styling, the main problem for me was that in using the example from the documentation, there is nothing visual which appears. It took me quite a while to realize that the handles are hidden by default.

I understand this is largely a result of how it was used prior to this pull request, but it seems to me that as a generic component, there's no reason to hide those controls by default. I could imagine we might want to keep this behavior for blocks in a block list, but rather than styling in ResizableBox and in individual block stylesheets, it seems like something which should be managed in one of the stylesheets for the block list (apply consistently and exclusively to ResizableBox within blocks).

I could create an issue if it seems reasonable.

chrisvanpatten requested review from tofumatt and a team October 4, 2018 19:43

chrisvanpatten added [Type] Enhancement A suggestion for improvement. General Interface Parts of the UI which don't fall neatly under other labels. [Package] Components /packages/components labels Oct 4, 2018

tofumatt added this to the 4.1 milestone Oct 4, 2018

tofumatt requested changes Oct 5, 2018

View reviewed changes

tofumatt reviewed Oct 5, 2018

View reviewed changes

chrisvanpatten force-pushed the feature/resizable-box-component branch from b912210 to 339fda1 Compare October 5, 2018 19:13

chrisvanpatten force-pushed the feature/resizable-box-component branch from 339fda1 to 29afb11 Compare October 6, 2018 11:27

chrisvanpatten and others added 7 commits October 6, 2018 07:27

Introduce a ResizableBox component and apply to image/spacer blocks

aa1acad

Add a shared class to all drag handles to simplify CSS selectors

c847d00

Add additional documentation and an example

c06e8e9

chore: Tweak docs

3d00599

Add changelog comment and update readme

d3dc6d7

Removed a stray "the"

7f07c4e

s/5.0.0/4.1.0/g

29afb11

chrisvanpatten mentioned this pull request Oct 7, 2018

Change in undo behavior breaking undo e2e test #10380

Closed

tofumatt approved these changes Oct 7, 2018

View reviewed changes

chrisvanpatten merged commit eafd620 into WordPress:master Oct 7, 2018

chrisvanpatten modified the milestones: 4.1, 4.0 Oct 7, 2018

aduth mentioned this pull request Oct 12, 2018

Testing: Fail CI build if local changes exist #10234

Merged

aduth reviewed May 2, 2019

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce a ResizableBox component #10347

Introduce a ResizableBox component #10347

chrisvanpatten commented Oct 4, 2018 •

edited

tofumatt commented Oct 4, 2018

chrisvanpatten commented Oct 5, 2018

tofumatt left a comment

tofumatt Oct 5, 2018

chrisvanpatten Oct 5, 2018

chrisvanpatten commented Oct 5, 2018

tofumatt Oct 5, 2018

chrisvanpatten Oct 5, 2018

tofumatt commented Oct 5, 2018

chrisvanpatten commented Oct 5, 2018

tofumatt commented Oct 5, 2018 •

edited

tofumatt commented Oct 5, 2018

chrisvanpatten commented Oct 5, 2018

chrisvanpatten commented Oct 6, 2018

tofumatt commented Oct 6, 2018

chrisvanpatten commented Oct 6, 2018

chrisvanpatten commented Oct 6, 2018 •

edited

tofumatt commented Oct 6, 2018 via email

tofumatt commented Oct 6, 2018

chrisvanpatten commented Oct 7, 2018 •

edited

chrisvanpatten commented Oct 7, 2018

tofumatt commented Oct 7, 2018 via email

tofumatt left a comment

aduth commented Oct 10, 2018 •

edited

aduth commented Oct 10, 2018

chrisvanpatten commented Oct 10, 2018

aduth May 2, 2019 •

edited

chrisvanpatten May 2, 2019

aduth May 3, 2019 •

edited


		## Usage

		Most options are passed directly through to [re-resizable](https://github.com/bokuweb/re-resizable) so you may wish to refer to their documentation. Note that we set the `handleClasses` and `handleStyles` to enable the Gutenberg-specific styles.

Introduce a ResizableBox component #10347

Introduce a ResizableBox component #10347

Conversation

chrisvanpatten commented Oct 4, 2018 • edited

Testing

tofumatt commented Oct 4, 2018

chrisvanpatten commented Oct 5, 2018

tofumatt left a comment

Choose a reason for hiding this comment

tofumatt Oct 5, 2018

Choose a reason for hiding this comment

chrisvanpatten Oct 5, 2018

Choose a reason for hiding this comment

chrisvanpatten commented Oct 5, 2018

tofumatt Oct 5, 2018

Choose a reason for hiding this comment

chrisvanpatten Oct 5, 2018

Choose a reason for hiding this comment

tofumatt commented Oct 5, 2018

chrisvanpatten commented Oct 5, 2018

tofumatt commented Oct 5, 2018 • edited

tofumatt commented Oct 5, 2018

chrisvanpatten commented Oct 5, 2018

chrisvanpatten commented Oct 6, 2018

tofumatt commented Oct 6, 2018

chrisvanpatten commented Oct 6, 2018

chrisvanpatten commented Oct 6, 2018 • edited

tofumatt commented Oct 6, 2018 via email

tofumatt commented Oct 6, 2018

chrisvanpatten commented Oct 7, 2018 • edited

chrisvanpatten commented Oct 7, 2018

tofumatt commented Oct 7, 2018 via email

tofumatt left a comment

Choose a reason for hiding this comment

aduth commented Oct 10, 2018 • edited

aduth commented Oct 10, 2018

chrisvanpatten commented Oct 10, 2018

aduth May 2, 2019 • edited

Choose a reason for hiding this comment

chrisvanpatten May 2, 2019

Choose a reason for hiding this comment

aduth May 3, 2019 • edited

Choose a reason for hiding this comment

chrisvanpatten commented Oct 4, 2018 •

edited

tofumatt commented Oct 5, 2018 •

edited

chrisvanpatten commented Oct 6, 2018 •

edited

chrisvanpatten commented Oct 7, 2018 •

edited

aduth commented Oct 10, 2018 •

edited

aduth May 2, 2019 •

edited

aduth May 3, 2019 •

edited