Support from Acquia helps fund testing for Drupal Acquia logo

Comments

surendramohan’s picture

Assigned: Unassigned » surendramohan

I am looking into this issue.

jhodgdon’s picture

Assigned: surendramohan » Unassigned
Issue summary: View changes

I'm unassigning after several months with no activity. Open to anyone!

mfernea’s picture

I'm looking into this.

mfernea’s picture

Status: Active » Needs review
FileSize
1.68 KB

I'm posting a patch for this. Hope it's ok.

Xano’s picture

  1. +++ b/core/modules/edit/edit.module
    @@ -15,6 +15,23 @@
    +      $output .= '<p>' . t('The Edit module allows users with the <em>Access in-place editing</em> permission to make in-place content editing for fields. For more information, see <a href="!edit_do">the online documentation for the Edit module</a>.', array('!edit_do' => 'https://drupal.org/documentation/modules/edit')) . '</p>';
    

    "to make in-place editing" is not correct English. What about something like "to edit field content without visiting a separate page"?

    Also, !edit_do is not a very descriptive placeholder. I propose !handbook_url.

  2. +++ b/core/modules/edit/edit.module
    @@ -15,6 +15,23 @@
    +      $output .= '<dd>' . t('When enabled, rather than having to visit a separate page to edit content, it may be edited in-place. Once the user has the appropriate permissions, an edit button (a pencil) will appear in the top right corner of nodes and node teasers. Clicking this button allows the user to edit the field or delete the node.') . '</dd>';
    

    Replace "appropriate permissions" with the actual permission name.

    Don't describe what exactly the button looks like and what types of content it will be displayed on, as this may be changed or extended by contributed modules.

mfernea’s picture

Ok, thanks for feedback. Here is the new patch.

robcarr’s picture

Status: Needs review » Reviewed & tested by the community

Not much to say. Text seems fine and self-explanatory

Xano’s picture

Status: Reviewed & tested by the community » Needs review
+++ b/core/modules/edit/edit.module
@@ -15,6 +15,23 @@
+      $output .= '<dd>' . t('When enabled, rather than having to visit a separate page to edit content, it may be edited in-place. Once the user has the <em>Access in-place editing</em> permission, an edit button will appear which allows the user to edit the field\'s content.') . '</dd>';

If a string contains single quotes, but no double ones, use double quotes to wrap the entire string, so the single quotes do not have to be escaped. This makes work easier for translators, who often are not developers.

mfernea’s picture

New patch containing required modfications.

Xano’s picture

Great, thanks! Can you also upload an interdiff?

JurgenR’s picture

Status: Needs review » Reviewed & tested by the community

Pretty straightforward, clear information.

mfernea’s picture

FileSize
1007 bytes

I'm adding the interdiff.

Xano’s picture

Very good, thanks! RTBC from me too.

jhodgdon’s picture

Status: Reviewed & tested by the community » Needs work

I don't agree that this is quite ready to go.

The last sentence:

Once the user has the Access in-place editing permission, an edit button will appear which allows the user to edit the field's content.

This refers to "the field's content" but the antecedent saying what "the field" is, is way up in About. I think it's too far removed. Can we fix this to be clearer?

Also, does this module allow you to edit just textual content or are all field types supported? This should be explained -- people may not know (I don't know, for instance), and they may make assumptions one way or another unless the help is explicit.

I also think it's not quite as simple as the Uses statement implies. At least the last time I tried to use this feature, you had to sort of put the page into edit mode by clicking one button, and then go find the piece of content to edit. Has that been changed?

Thanks!

jhodgdon’s picture

OK, I just tested this with a D8 that was installed about a week ago, so I think it's reasonably up to date. The Edit module is enabled.

I really don't think this help text is very accurate compared to what I actually see when I try to use this feature.

So... using either the Bartik theme or Stark, here is how it actually appears to work:
- I have to hover over an area that comes from a node to see contextual links for that node.
- In the contextual links, I see choices of "Edit" (which takes me to the full edit page) or "Quick edit".
- If I choose Quick Edit, I get boxes around the pieces of the node that I can edit (title and body in this case when I'm on the /node page, but when I'm on /node/1 I am apparently unable to edit the title -- is that a bug???).
- Then if I actually want to edit one of those areas, I have to click the area first, and then an editor comes up. Even for a plain-text field like the title, it takes two clicks before I can edit the title.
- It doesn't seem to work for the comments. That needs to be explained in the help because the help as it is now seems to imply it works for any content.

mfernea’s picture

Ok, I'll update the help text.

batigolix’s picture

To use in-place editing a user has to choose the Quick edit option from the Contextual links. The editable "regions" are marked with a blue border. After clicking on a editable "region" the in-place editor becomes available.

In-place editing seems available on Long text fields in nodes. But not on other text fields.

In place-editing does not depend on the text format. So it is also available for plain text fields.

It works in different view modes (full, teaser)

I do not get the in-place editing option in long text fields in Blocks, Comments or Users

In some demo's on Youtube they show how all kind of text fields can be edited in-place, but I cannot reproduce that with a fresh D8 install.

I'll ping Wim Leers to see if he can confirm what the Edit module can do at this moment.

More info:
https://drupal.org/project/edit
http://www.youtube.com/watch?v=BVxhWJdZ7vY
http://drupalize.me/blog/201310/drupal-8-wysiwyg-and-line-editing

batigolix’s picture

Component: documentation » edit.module
jhodgdon’s picture

RE #17 - When I tried it from the /node page, the title field was editable. When I tried it from /node/1, the title field was not editable. So it doesn't seem consistent.

I did some more testing and I agree with your assessment about it not being available for User pages and Comments.

However, I also tested multiple fields on a node. I added pretty much every type of field to the Article content type, and edited to give them all values. When I clicked the "quick edit" link, all of the fields were editable: images, taxonomy terms, short text, long text, telephone, email. It doesn't seem to depend on the field type.

For all the field types, I had to click once on the box to "activate" the editing, and then again in the editable area to actually edit the value. This is kind of annoying, especially for text fields where you would think they could get the focus into the field. Probably we should file an issue on that.

But anyway... It does look like it works for everything except the title and the "Posted on" information for nodes, but it doesn't work on other entity types.

batigolix’s picture

Status: Needs work » Needs review
Parent issue: » #1908570: [meta] Update or create hook_help() texts for D8 core modules

Attempt to stuff all this information in the help text:

About

The Edit module allows users to edit field [LINK] content without having to visit a separate page. This "in-place editing" is available for main site content but not for comments, custom blocks, taxonomy terms and other entity types [LINK] . In-place editing is possible in both teaser and full display mode of the content. For more information, see the online documentation for the Edit module [LINK].

Uses

Editing content in-place

To activate in-place editing, hover the content and choose "Quick edit" from the contextual links [LINK]. A blue border appears around the fields that can be edited. Click on the field to start editing its content. A visual text editor will be available if the text format of the field is linked with it.

In-place editing is only available for users that have the "Access in-place editing" permission [LINK].

Not sure how to get the wording right in the About section: "This "in-place editing" is available for main site content but not for comments, custom blocks, taxonomy terms and other entity types [LINK] . In-place editing is possible in both teaser and full display mode of the content. "

My idea was to explicitly explain the term "in-place" editing and mention its availability in certain entity types.

jhodgdon’s picture

Title: Update hook_help for Edit module » Add hook_help for Edit module
Priority: Normal » Critical

I think that this text looks reasonable, and it reflects what I have seen happening in the UI. One clarification: I think it's actually not limited to teaser and full display modes, but we should probably test that.

I'd also like to see some feedback from the maintainers of this module (Wim Leers and nod_) about whether it is supposed to apply only to node content or if there is something else going on that is only temporarily preventing it from applying to comments, blocks, taxonomy terms, user accounts, and other entities. I'll ping them for feedback since they have not responded to this issue yet.

I'm also updating the title/status. This module was committed without hook_help() in violation of the core gates, so creating hook_help for it is a critical issue.

batigolix’s picture

Regarding your remark

 I think it's actually not limited to teaser and full display modes, but we should probably test that.

Yes, in-place editing is available in *all* display modes. That is what I intended to say

Wim Leers’s picture

  1. When I tried it from the /node page, the title field was editable. When I tried it from /node/1, the title field was not editable. So it doesn't seem consistent.

    This is because of Drupal's pretty insane/crappy way of rendering things: when looking at the "full page" of an entity, the "title" field is NOT rendered, but the page title is dynamically modified to be the field title. Which means that the data- attributes that are set on the title field are lost. That's a Drupal core bug that still needs to be fixed.

  2. It doesn't yet work for the "created" and "author" "fields" because they're not "configurable fields", they're "base fields". That will be made possible in #2010930: [META] Apply formatters and widgets to rendered entity base fields (all besides node.title). It took the Entity/Field API guys more than 6 months (maybe even a full year, I don't remember), to do the necessary refactoring to make titles in-place editable. Now the groundwork is in place, and #2010930 will make all fields, configurable or base, in-place editable. But I was asked by the Entity/Field API folks to wait with working on that because they wanted to do some other work first.
  3. In-place editing of custom block entities will soon work again, once #2171269: Handle block rendering's #attributes correctly (fixes in-place editing of custom blocks) lands.
  4. It works in any display mode. And in any language.
  5. If you add fields to your taxonomy term entities, then those fields will also become in-place editable. Generally, any entity with fields that has contextual links is in-place editable.
jhodgdon’s picture

Status: Needs review » Postponed

I think we need to postpone this until the Edit module is working as it is soon supposed to work, so that those reviewing/writing the help can see it in action and write appropriate help.

catch’s picture

Status: Postponed » Needs review

Neither #1 nor #2 look like they would need to be explicitly mentioned in a hook_help(), or prevent one being written.

jhodgdon’s picture

Well, maybe. It is just really difficult to write coherent, readable, sensible help when the module doesn't behave in a sensible manner. Good luck.

catch’s picture

The reason this is critical is because edit module was committed with no hook_help() at all. Had that been included at the time, then the help added at that time would not have been able to deal with any of the issues in #23. So we might not be able to add comprehensive help here, but the issues this was postponed on aren't even critical tasks/bugs- you can still edit entities via the full form if we end up missing something and we might end up releasing like that.

jhodgdon’s picture

OK, well someone needs to try to make sense of this module and put it into a help text, and then we should definitely file a follow-up to review the help before the release to see if it still matches the behavior of the module, because there is a good chance that as the UI is updated, the hook_help won't be (based on past experience of D6, D7, D8 development).

jhodgdon’s picture

Status: Needs review » Needs work

IMO this should be the responsibility of the module maintainers at this point, because only they are probalby in a position to know what the module currently does and how to make sense of it. So please Wim Leers or nod_, try to make a new patch that matches what the UI does and makes sense to a non-programmer person. Thanks!

catch’s picture

Issue tags: +Spark

Tagging.

Wim Leers’s picture

#23.1 is being fixed at #2216437: Entity labels are not in-place editable on "full entity page" (prime example: node title), and has been ready to go for weeks.

#23.2 is being fixed at #2226493: Apply formatters and widgets to Node base fields, and has only one remaining test failure to be fixed.

webchick’s picture

Issue tags: +Novice

As far as criticals go, this would be an easy one to pick off.

jhodgdon’s picture

Title: [PP-2] Add hook_help for Edit module » [PP-2] Add hook_help for Quick Edit module

Module name changed to 'quickedit.module' by the way.

jhodgdon’s picture

We just had a change to hook_help, on this issue: #2183113: Update hook_help signature to use route_name instead of path.

Here is the change record: https://drupal.org/node/2250345

This patch will need a reroll for this change.

filijonka’s picture

Status: Needs work » Needs review
FileSize
2.65 KB

just trying to get a first sence of what this should be about and first step to get an acceptable text and then worry about the structure of the text. this is not a reroll on any existing patches but a rewriting from scratch.

jhodgdon’s picture

Status: Needs review » Needs work

The About section from the previous patch (#9) was good (aside from the module name changing). Please go back to that patch and get it. Thanks!

Some other issues:
b) The module's name is "Quick Edit", not "Quickedit".

c) When referring to a module, capitalization should be ... the Quick Edit module ... always.

d) Do not use the url() function. See Drupal 8 note at the bottom of the Help Guildelines page: http://drupal.org/node/632280

e) There were a lot of typos in this text... but really let's go back to basics. In Uses, we need to describe what this module is used for. It seems like there should just be one DT/DD item here, called "In-place content editing", because that is what the module does. I don't think we need to describe the UI in such detail. We just need to tell where it will be shown, tell how to activate the editing (which isn't obvious), and leave the rest to the UI being fairly obvious.

And as a general note, do not use visuals like "pencil button" in your description. The appearance of this button is dependent on the theme, and also blind users would not see it as a "pencil" at all. So figure out what the button text is, and call it that (like "the Edit button" or "the Activate button" or whatever it is), with a parenthetical note saying something like "(which looks like a pencil in the core themes)".

xjm’s picture

Component: edit.module » quickedit.module
amitgoyal’s picture

Status: Needs work » Needs review
FileSize
2.17 KB
3.34 KB

@jhodgdon - I have made all the changes in a, b, c, d, e points.

Please review.

jhodgdon’s picture

Status: Needs review » Needs work

Thanks, this looks much better!

A few minor things to fix:

a) Before any UL list, the preceding line should end in :
So:

two ways of activating the in-place content editing.

the . at the end should be :

b) Does this work? \Drupal::url('user.admin_permissions', array(), array('fragment' => 'module-path') It looks like it would be sending them to admin/people/permissions#module-path, which cannot be useful?

c)By hovering the content
This needs to be "By hovering over the content"... Also, the text above is talking about editing fields, not editing content. So maybe saying "By hovering over the content generated from the field..."?

d) By clicking the Edit button
This is only active if the Toolbar module is enabled, right? In that case we should mention that and link to the Toolbar help.

e) The "two ways to activate the in-place editing" are really two ways to make the contextual links visible, it seems? or maybe it's a way to make the contextual links activator button (which looks like a wheel) visible? I am not sure.

f) What does this mean?

Clicking the contextual link there will be an option "Quick Edit" if available on that content. Choosing that all editable fields will be surronded by a blue border and the current active field will also have show a popup for that field.

This is not accurate and it is confusing...
- There is not one "contextual link", there is a list of contextual links, and you are clicking the wheel that shows the list, right?
- "on that content" should be "for that content", and I think we need a comma before "if available".
- I just don't understand the last sentence at all. Doesn't this happen when you do the pencil icon, but not when you do the hovering over a particular item? What is the popup? I really don't get it.
- As I've tried to say before, things like "blue border" are not helpful for blind people, and are dependent on the theme. We need to figure out a way to describe these visual attributes that does not involve being that specific or that visual.
- "surrounded" is misspelled.

amitgoyal’s picture

Status: Needs work » Needs review
FileSize
2.47 KB
2.57 KB

Thanks for your feedback. I have made the below changes,

a) : has been added at the end
b) Although array('fragment' => 'module-path') was working but I have changed it to array('fragment' => 'module-quickedit').
c) Text changes as suggested
d) Reference and link added for Toolbar module
e) Revised the text for more clarity and corrected the spellings.

Please review.

jhodgdon’s picture

Status: Needs review » Needs work

So... I still do not think this text is very clear. We're talking about the Quick Edit module, and in Uses, it starts off talking about the contextual links activator button... uh... huh? We need to first explain why someone should care about contextual links and how they are related to Quick Edit. I also tested this module out again to see how it currently behaves and I think I understand better what you are trying to say.

And by the way, our help guidelines say Uses headings should start with verbs.

So here's a suggestion for Uses:

Editing content in-place

To edit content in place, you need to activate quick edit mode for a content item. Quick edit mode is activated via the (link to contextual links help)contextual links(endlink) button (which looks like a pencil in most themes, and is normally displayed in the upper right corner of the content area). There are two ways to make the contextual links button visible:
- Hovering over the area where the content is displayed will temporarily make the button visible.
- If you have the (link to toolbar help)Toolbar module(endlink) enabled, clicking the contextual links button in the toolbar (which looks like a pencil) will make all contextual links on the page visible. Clicking this button again will toggle them to invisible.
In either case, click the contextual links button for the content you want to edit, and choose Quick edit to activate quick edit mode.

Once quick edit mode is activated, you will be able to edit the individual fields of your content. In the default theme, with a JavaScript-enabled browser and a mouse, the output of different fields in your content is outlined in blue, a pop-up gives the field name as you hover over the field output, and clicking on a field activates the editor. Closing the pop-up window ends quick edit mode.

jhodgdon’s picture

One other note: On #2091311: Update hook_help for Contextual Link module I learned that users need "Use contextual links" permission to see contextual links. It may be helpful to mention this in the help for Quick Edit as well, and also mention that entity or field permissions may also be necessary?

And... Really I think this help about the contextual links should do into the Contextual links module help and not here.

So probably we should ignore my suggestion in #41 and instead say something like this:

To edit content in place, you need to activate quick edit mode for a content item. Activate quick edit mode by choosing Quick edit from the contextual links for an area displaying the content (see the (link)Contextual Links module help(endlink) for more information about how to use contextual links).

Once quick edit mode is activated, you will be able to edit the individual fields of your content. In the default theme, with a JavaScript-enabled browser and a mouse, the output of different fields in your content is outlined in blue, a pop-up gives the field name as you hover over the field output, and clicking on a field activates the editor. Closing the pop-up window ends quick edit mode.

amitgoyal’s picture

Status: Needs work » Needs review
FileSize
2.08 KB
2.99 KB

The revised text looks good. I have made the changes in Uses section as per your feedback in #42.

Please review.

jhodgdon’s picture

Status: Needs review » Needs work

Close!

There's an extra ) here: ...Links module help</a>) for...

Also I still think we need to mention the contextual links permissions. I do not think someone without contextual links permission can activate Quick Edit, right? And you would also need to have permission to edit the content and its fields?

amitgoyal’s picture

Status: Needs work » Needs review
FileSize
2.39 KB
2.83 KB

Thanks for your feedback in #44.

I have removed the extra bracket ). As 'Access in-place editing' was already mentioned in About section so I have also specified 'Use contextual links' permission there as both are required for quick editing.

Please see if it makes sense.

mparker17’s picture

Title: [PP-2] Add hook_help for Quick Edit module » Add hook_help for Quick Edit module
Status: Needs review » Reviewed & tested by the community

Changing title: not postponed anymore.

All the links work. Help text makes sense, terminology matches the interface and the Quick Edit functionality works as described.

I think this is RTBC!

jhodgdon’s picture

Status: Reviewed & tested by the community » Fixed

Thanks all! Committed to 8.x.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.