Postponed by:

Problem/Motivation

When block place module is enabled and outside in edit mode is turned on if the user clicks "Place block", the configuration form appears in a modal. If, after placing the block the user clicks the block the same configuration form appears in the off-canvas tray.

Proposed resolution

When both outside in and block place are enabled use the off-canvas tray for the place block form.

Remaining tasks

Javascript tests for sorting functionality

When placing a block in region with no other blocks don't open the tray to sort blocks(there will be only 1)

Ok here is patch that changes block_place to use offcanvas if outside_in is enabled. This should probably be changed the "block_place" component when available.

It currently works(I like this much better!) but probably should add some js/css to highlight which region the user just clicked. In the modal it doesn't really matter as much but with offcanvas it would be a nice change. Should that be a follow-up?

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

SKAUGHTCreditAttribution: SKAUGHT as a volunteer commented 21 April 2017 at 16:16

Status:

Needs review

» Needs work

simplytest.me under 8.2
seems to work well.

it does require the outside in to be active as well. i was expecting block place to just be in the tray (but i'm personally unsure of how much the tray codebase is in core) and if it could be called without outside in being active.

Scenario 2: In the event that Outside In doesn't stabilize, we remove and perform some cleanup to work with a modal window. Changing data dialog type to "modal" instead of "off_canvas" (this is a small tweak).

I clicked "Place Block", clicked "+" in a region, selected the block, filled out the settings and clicked save.

The page reloaded and the block sorting popped up for the selected region. If I clicked "save", it reloaded the page and the block was placed where desired. If I attempted to re-order the blocks before saving, a JS error occurs and I can no longer save.

The implementation is spot on. The sorting form, implemented as a separate form, would allow for easily "adding a second icon next to or below the plus in regions with > 1 block to allow the blocks to be sorted" as @pwolanin notes.

I am marking this as "Reviewed & tested by the community". I did see, what seems to be, a minor indentation code standard violation in the `block_place_page_top` part of the patch. And, @tedbow, I'd be curious to get your thoughts if this needs any additional automated tests.

In the last meeting I had with the UX we had decided that there should be a new contextual link on each block that says "reorder blocks"(or similar). This would open the tray with the blocks in that region to be sorted. This way you won't have to click "Place Block" when you actually want to sort. It would also give you the ability to sort blocks outside of placing. I was going to add this functionality to #2784495: Normalize block place and outside-in experiences

This patch also adds the new es6.js file needed now for all Javascript

Fixes Code standards mentioned in block_place_page_top

UPDATE:
I had thought about not putting the contextual link in regions that only have 1 block because it doesn't make sense to sort. But I thought that in some themes it would not be obvious 1 one region started and another ended so the new site builder or someone new a particular theme it would not be clear why some blocks had the "sort" link and other didn't.
For instance I think actually in Bartik Footer 1, Footer 2 would be confusing.

I did just realize we could not open the tray for sorting at the end if we are placing a block in a region with no other blocks. It doesn't make sense because they can't do anything.

It might be worthwhile showing the machine name in a new column (or on hover) to highlight the block instance differences. That, to me, is a block instance differentiation. There may be other less technical attributes we can show.

I would assume our expected release is 8.4.x. I'm going to update the issue accordingly and confirm the patch applies cleanly. Apologies in advance if this is indeed 8.3.x, for which I'm happy to reroll #40 if needed.

Why selector is '.ckeditor' prefix? Can it work without CKEDITOR module?

OutsideIn doesn't work:

1. node?block-place=1&destination=/drupal8x/node
2. click `+` to open Place block (and then do nothing)
3. click top left Edit to open Setting Tray
4. click any `Setting Tray` area to edit. (setting tray dialog does not shown)

As of the patch found in #50, hook_page_top is being used to load the JS library needed to sort blocks.

This may be appropriate for the timing of when the JS library should be loaded on the page. However, in my opinion, the context seems off to me. The sort form loads in either a modal or in the settings tray, depending on if the settings tray module is enabled. Neither of these relate to my understanding of hook_page_top.

One alternative would be to consider something like hook_page_attachments, which would more generically load the library and can perform the contextual check for "block-place-region-sort".

Adding a patch that switches this logic to hook_page_attachments for review and consideration.

The only other change I made was #52.1
I think this was a copy/paste error of my from earlier in the patch

Re #52.2
Yes libraries files will never reference the *.es6.js files. The new es6 development process allows us to write in es6 Javascript but then use BabelJS to compile it to regular Javascript that the browsers can understand.

I think the change #51 to use hook_page_attachments is more correct because that is exactly what we are doing, altering attachments.

I don't think depending on the query string for state is great idea. Could we possibly add something at load `Drupal.blockPlace.isBlockPlaceMode = true`. With that we could get rid of this entire function.

@tim.plunkett I addressed the ones you mentioned in #62 as well as the ones I could do quickly while making the patch.
1. NOT Addressed
2. Yes at this point $destination will equal something like "/d8_2_ux/node?block-place-region-sort=content"
block-place-region-sort is passed in the query string to know which region the block was placed into so we can sort it.
The tray will open to sort this region on this page load. But we want the new destination for submitting the region sort for we don't want to pass block-place-region-sort again or the tray will open again after the user has already sorted.

5. fixed
6. fixed
7, fixed
8. fixed
9. "Thing" is very broad term to declare that some"thing" isn't. But fixed.
10. I didn't that was a "thing". fixed.
11. fixed
12. NOT addressed

14. See 2

15. Yes I guess this left over from when we only needed to use block_place.admin_library if we were using the tray and not a modal. but now with the extra sort step regardless of what dialog type we are using we always need to use block_place.admin_library. fixed.

16. NOT addressed

I also had started a JS test yesterday but it was not working for some reason. I have put a return statement to skip most of the test and have it pass for now. You may want to just scrap what I have there.

First time I see a width being specified? Seems inconsistent. Especially if we only do this if outside_in is installed. Oh and actually, it won't have any effect, since the width of the Settings Tray is pre-determined?