Closed (won't fix)
Project:
Edit
Version:
7.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
14 May 2013 at 07:53 UTC
Updated:
22 Jan 2014 at 18:29 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
saltednutOh - it was missing the invisible wrapper for when edit.css isn't loaded.
Comment #2
saltednutUgh.. seems I did not include my changes to edit.css in the last patch. :(
Comment #3
steven.wichers commentedNavbar module is using drupal-navbar as the class name. See:
#1757466: Prefix all navbar classes to prevent theme clashes
#1938044: Prefix all toolbar classes to prevent theme clashes
Images also did not get created from the patch file.
Comment #4
saltednut@steven.wichers how are you applying the patch? We are using it in the Demo Framework make file and not seeing issues with the images being created.
Comment #5
steven.wichers commentedcd path/to/edit
patch -p1 < path/to/patch
Comment #6
saltednut@steven.wichers does this work to apply?
rm -r path/to/edit
git clone --branch 7.x-1.x http://git.drupal.org/project/edit.git path/to/edit
cd path/to/edit
git apply --index path/to/patch
Comment #7
steven.wichers commentedThat worked with the following non-critical warnings:
Must be an issue with patch and creating new files.
Original problem remains: The navbar module is no longer using .navbar.
Comment #8
saltednut@steven.wichers No thats just bad patch writing on my part. You can ignore the EOF warnings.
This patch is just a bandaid really. I created it as a stub for the maintainers to hopefully work off of. If you want, please assign the issue to yourself and fix the css errors. Cheers!
Comment #9
saltednutCouldn't wait... :) @steven.wichers this should fix.
Again though, we need to keep working on this so that it works without having the block present on the page.
Comment #10
steven.wichers commentedUntested, but from browsing the code it seems as though all that is required is adding
'#attached' => array('library' => array(array('edit', 'edit')))to the page render array.Comment #11
saltednutOk, thanks for the tip! I've made some adjustments and this is working on my end even without the 'Quick Edit' block on the page.
Comment #12
saltednutOops - title is also required.
Comment #13
saltednutForgot to add the
--binaryflag in #12 to include the files. :\Comment #14
Poieo commentedThis is working well for me. I think the only thing I would add is:
A little more user friendly.
Comment #15
saltednut@Poieo - that is inconsistent with D8 and Responsive Preview - the button icon should actually turn white on hover:
Comment #16
Poieo commentedHmm... I guess since this is a 'button' and not a link, it's not picking up the css from navbar.theme.css on line 17:
I was more concerned with the fact that the cursor doesn't turn to a pointer on desktop devices like the rest of the menu items in navbar do.
Comment #17
saltednutThat's a good point. Hmm... I was basing this implementation off the Responsive Theme Preview module's implementation of hook_navbar.
-- http://drupalcode.org/project/responsive_preview.git/blob/refs/heads/7.x...
Might be more of a problem for the navbar module to fix. hook_navbar should be adding these classes, no?
Comment #18
greg__ commentedCan you please attach the image generated by the patch to this issue ?
For an unknown reason, my linux distrib didn't generate them when i applied the patch.
Thank you.
Comment #19
Poieo commentedAttached.
Comment #20
wim leersThanks for working on this, and sorry for not getting back to you sooner.
The Drupal 8 backport (#2111667: Backport Drupal 8's Edit module to Drupal 7) has fixed this — there's no more "Quick edit" link to start in-place editing ALL the things on the page, it now uses contextual links, just like in Drupal 7, hence this is a non-issue now :)
Comment #21
jessebeach commentedI know this issue is closed. I need a place for an interim patch that makes the version of Edit included in the Demo Framework distro work with the latest Navbar code. In other words, just ignore this patch :)
Comment #22
saltednutYes, its a zombie thread... We continue to roll with an earlier version of Edit in DF due to CKEditor packaging issues. This patch gets us current with Navbar 1.2 for the cf62974 revision of edit. (Please ignore.)
Comment #23
saltednutComment #24
wim leersA zombie thread :D
Comment #25
saltednutI think there is still some use case for this. At the very least, could we work on a backport of D8's functionality that highlights contextual links? I'd be happy to open another issue if that is necessary.
From my experience, and in talking to others, we very much found the "Pencil" icon in the navbar as a good place to start for inline editing. I am not sure how much work it would be to either refactor this patch or work on a new solution.
For our purposes, just fixing this patch to work with the latest Edit is a good start. I am attaching a non-working patch here that applies to HEAD. It brings in the navbar button and everything is theme'd correctly.
Just asking for some guidance as to what methods to invoke to put this together for demo/PoC purposes.Wim pointed me to #2172989: Is it possible to print a custom link to quick edit? in chat, so we can use that documentation to work on this PoC button. But it will likely only be in place for editing one node at a time. In the case where multiple entities are shown on a single page, the button will not be available and contextual links will have to be used.
Comment #26
wim leersI'm sure it's possible, but it might require overriding all of Contextual Links' JS in D7…