jessebeach’s picture

17.77 KB

This patch introduces the pencil icon as a universal edit trigger.

Contextual links are hacked so that they will exhibit their normal behavior until the edit mode is toggled on. In edit mode, the contextual link triggers are always present on the page.

webchick’s picture

From the 8.x-1.0-unifiedEdit1 tag, drush make is failing on:

Unable to patch drupal with 1898020_unified-edit_1.patch.            [error]
Unable to patch drupal with                                          [error]

However, the tip of the unified-edit branch seems to work ok.

webchick’s picture

Note that I can't cut a release of Spark with this in it unless it's merged into the main 8.x-1.x branch.

webchick’s picture

I can see this working successfully on but when building with Drush manually, I don't see pencils in the circles for some reason:

No pencils!

Additionally, on the demo site the search box inexplicably does not have one of these hover links. Is it a missing permission on the tester user? If that's not easy to fix, no worries, I think we can go ahead with testing and just call that a bug.

WOOT! Thanks, Jesse!!

Wim Leers’s picture

@Jesse: in your patch at #1, you forgot to include core/misc/edit.png in the patch — this is what causes #4 :)

Gábor Hojtsy’s picture

I assume core/modules/edit/images/icon-edit.png is the same as should be at core/misc/edit.png so it can be copied over.

Wim Leers’s picture

Yes, essentially the former is moved to the latter, and that is all that changes in that area. (This allows the Contextual Links module to refer to the same image.)

Gábor Hojtsy’s picture

BTW trying to apply this to Drupal 8 current; looks like you rolled this against other patches already applied.

$ patch -p1 < 1898020_unified-edit_1.patch
patching file core/modules/contextual/contextual.js
Hunk #1 succeeded at 9 with fuzz 2 (offset 2 lines).
Hunk #2 FAILED at 18.
Hunk #3 FAILED at 56.
Hunk #4 FAILED at 149.
3 out of 4 hunks FAILED -- saving rejects to file core/modules/contextual/contextual.js.rej
patching file core/modules/contextual/contextual.theme.css
Hunk #1 succeeded at 7 with fuzz 2 (offset -6 lines).
Hunk #2 FAILED at 15.
Hunk #3 FAILED at 43.
Hunk #4 succeeded at 68 with fuzz 2 (offset -6 lines).
Hunk #5 succeeded at 85 (offset -6 lines).
2 out of 5 hunks FAILED -- saving rejects to file core/modules/contextual/contextual.theme.css.rej
patching file core/modules/edit/css/edit.css
patching file core/modules/edit/js/createjs/editingWidgets/drupalcontenteditablewidget.js
patching file core/modules/edit/js/createjs/editingWidgets/formwidget.js
patching file core/modules/edit/js/views/menu-view.js
patching file core/modules/edit/js/views/propertyeditordecoration-view.js
patching file core/modules/edit/js/views/toolbar-view.js

Gábor Hojtsy’s picture

Title: Get unified edit patch working » Proof of concept for unified edit pencils
Wim Leers’s picture

- This is against Spark D8, not D8 HEAD.
- Jesse is applying another patch first to contextual.module, hence the failure:
- Then, apply this patch.
- This patch does ship with the core/misc/edit.png file that was missing icon file because ***I*** failed to include it properly in my original "pencils patch".

Wim Leers’s picture

#2: Indeed, @jessebeach simply appears to have forgotten to update the tag to point to the tip of the branch — though I doubt d.o allows removal of tags…, so she probably couldn't have changed it.

A major problem: getting the f*********ing core/misc/edit.png file to work:

drupal patched with contextual-contextual_links_keyboard_accessibility-849926-60.patch.                                                                                                                                            [ok]
drupal patched with unified-edit-prototype-demo-37.patch.                                                                                                                                                                          [ok]
drupal patched with 1898020_unified-edit_10.patch.                                                                                                                                                                                 [ok]
Generated PATCHES.txt file for drupal

All is well, yet the PNG file is still missing. Argh?

Gábor Hojtsy’s picture

Applying first and then #10, it applies cleanly to Drupal 8 core. Here is a combined patch just for human-testing. Just applying #10 itself, the edit png appears for me. Not sure why it would not work in a make scenario like what Wim showed.

Gábor Hojtsy’s picture

149.8 KB

Testing feedback based on that patch on top of vanilla D8:

- Contextual links still "work" "somewhat" without edit module but only on the first click or so. Any further clicks just make the menu flickr and disappear immediately. Also the blue border that seems to show up with the edit mode flickrs up for a split second.

- With the edit mode turned on it looks like this:


Wim Leers’s picture

#13: all the Edit outlines are by design, that's how it always worked. The outlines around the contextual link-ized blocks are new. And yes, the in-place editors used by Edit are always top-left aligned (in LTR at least), which is where the actual content is rendered as well.

Gábor Hojtsy’s picture

Wim: I understand Edit already works so it displays the edit widget on the left, however, now the pencils are on the far right, which makes them disconnected. That is not a bug report, but rather usability feedback :)

However, here is a bug report with z-indexes of the pencils:


Also another attempt at a binary patch roll (against Drupal 8), however I could not figure out how to roll the nice ascii looking patches that Jesse produced.

Gábor Hojtsy’s picture

60.21 KB

If the pencil's z-index is lowered to 499 (vs. the overlay's dim layer's 500), then it looks great.


jessebeach’s picture

The pencil icons will also display above the field edit modal, which is at z-index 310. There's still a little more fine-tuning to do here to get everything on a the right plane.

Gábor Hojtsy’s picture

Status: Active » Closed (duplicate)

Closing down as duplicate of #1874664: Introduce toolbar level "Edit" mode that shows all contextual links. Please post updated work there.