Problem/Motivation

While more closely reviewing the wording of the QE help topic before committing it to contrib at #3264949: Move Quick Edit help topics to contrib Quick Edit module, and locally testing to follow along, I noticed a couple of bugs in the wording. See https://git.drupalcode.org/project/quickedit/-/merge_requests/6

  1. "as well as permission to edit the particular content." --> what about the Access in-place editing permission from quickedit.permissions.yml? They need that, too, right?
  2. "For example, if you want to edit the content of a block..." --> Blocks are config, not content. Only settings_tray lets this example work. We need to say something like:
    Find the contextual link for the part of the page you want to edit. For example, if you want to edit a specific post on the front page, the link should be in the top-right corner of the post, or top-left for right-to-left languages.
  3. "An editing form for the content should appear on the page." --> Not exactly. In local testing, it looks like once you click Quick edit every editable field gets a blue box, you can hover over each one and it tells you what the field label is. If you click on a field, it lets you edit that field. Kinda clumsy to describe in words. Maybe what we have is sufficient. But perhaps this could be improved? Maybe no one cares. 😉

Steps to reproduce

  1. Use 9.4.x branch of core to install a test site.
  2. Enable help_topics, quickedit but not settings_tray modules.
  3. Visit the "Using in-line (quick) content editing" help topic (core.quickedit).
  4. Try to follow the instructions.

Proposed resolution

Fix them.

Remaining tasks

  1. Do it.
  2. Reviews / refinements.
  3. RTBC.
  4. Commit.

User interface changes

API changes

Data model changes

Release notes snippet

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dww created an issue. See original summary.

dww’s picture

Title: Fix inaccuracies in quickedit help topic (now that its split from settings_tray) » Fix inaccuracies in quickedit help topic (after the split from settings_tray)
Assigned: Unassigned » dww
Issue summary: View changes

Working on a patch.

dww’s picture

Assigned: dww » Unassigned
Status: Active » Needs review
Issue tags: +Bug Smash Initiative
FileSize
2.93 KB

I don't love this text for the 3rd point about how quickedit itself actually works. Maybe this is too much detail. But here's a draft to get us started:

Click the link to open the contextual links menu, and click Quick edit. All editable fields will be enclosed in a light blue box. Hover over any box to see the label of the field that box contains. Slick on the box and a form to edit that field will appear.

p.s. I wish there was a "try to apply it and check for code style / cspell, but don't bother running any real tests" option when uploading patches like this. 😉

Spokje’s picture

p.s. I wish there was a "try to apply it and check for code style / cspell, but don't bother running any real tests" option when uploading patches like this. 😉

Run All PHPUnit Tests The Way The Testbot Does It

and/or

core/drupalci.yml:

build:
  assessment:
    testing:
      # Run code quality checks.
      container_command.commit-checks:
        commands:
          - "core/scripts/dev/commit-code-check.sh --drupalci"
        halt-on-fail: true
dww’s picture

FileSize
2.93 KB
1.44 KB

Hah, you'd think even after copy/pasting it into the comment, I'd have caught my own typo. Glad I re-read this. s/Slick/Click. Yeah, yeah, I know #3265073: Re-word all admin e-mails to remove "click", but the rest of the topic is full of "click", so I'm sticking with it. 😉

dww’s picture

@Spokje Indeed, I run ./core/scripts/dev/commit-code-check.sh myself, but committers want to see the results from the bot. And if I hack drupalci.yml (which I do regularly) core committers can't actually commit the patch without reverting the drupalci.yml change (which they're reluctant to manually do any such thing during a commit, for completely justifiable reasons).

Spokje’s picture

Ah, I somehow thought you didn't know about drupalci.yml abuse.
(Which would be pretty weird since you've been around for a bit, come to think of it).

Never mind, move along, nothing to see here...

catch’s picture

Blocks are config, not content.

This isn't the case for custom blocks - those are content entities, and should be editable with quickedit I think.

quietone’s picture

Yes, custom blocks are editable with quickedit. However, the change of the example to a piece of content on the front page is probably a bit better, easier to understand and follow, than one referring to a custom block which the site may not have.

I applied the patch and then went hunting for the page. I could not find it at /admin/help where none of the topics use the word 'edit'. I found it by using the text of one of the related items in quick_edit.html.twig in the URL. How is a user supposed to find this? Or am I being daft on a Friday evening? ... Pause I just found it. It is under 'Using the administrative interface'.

Once I got there I read the text and followed the steps to edit an article on the front page listing. The emphasis is used correctly, ready well and it worked.

The changes look fine to me. Is there anything else to do here?

dww’s picture

Issue summary: View changes

Great, thanks! Nothing more but RTBC & commit afaik.

Spokje’s picture

Status: Needs review » Reviewed & tested by the community

Here's the RTBC part.

(Agreeing with @quietone's comment about the help topic being hard to find, but that's a possible follow IMHO)

Amber Himes Matz’s picture

Issue summary: View changes
Status: Reviewed & tested by the community » Needs review

Ok, I applied the patch in #5 and it created core/modules/quickedit/help_topics/quickedit.quick_edit.html.twig.

This is the file that will exist in Drupal 10. But in Drupal 9, all core module help_topics should still be in core/modules/help_topics/help_topics. At present this issue is labeled with version 9.4.x-dev. Is that an error?

My understanding is that this issue is only to correct the text. Am I applying the wrong patch?

To not-really-answer-but-explain the "findability" question, there are currently 3 ways to find help topics:

1. Browse the top level topics on admin/help (topics with top_level: true).
2. Click on a related link from a top level.
3. Use the search.

We do have an issue about making topics organized differently to help with findability:

https://www.drupal.org/project/drupal/issues/2960220

Regarding the standards question, our standards are outlined here:

https://www.drupal.org/docs/develop/managing-a-drupalorg-theme-module-or...

Which recommends following these UI text standards here:

https://www.drupal.org/drupalorg/style-guide/content

I'm not saying that all the help topics are in perfect alignment with those, so if there are issues, feel free to add follow ups. I cringe a bit at the use of "click". I prefer "select" as per Microsoft's style guide. But "click" is in the Drupal UI text guidelines at present.

Finally, I'm updating the issue summary's steps to reproduce to include enabling Help Topics.

Sorry, but setting back to needs review until the file placement and is this issue filed against the correct branch question are answered.

Amber Himes Matz’s picture

Status: Needs review » Needs work

Actually, I think it needs work if the file is in the wrong place.

dww’s picture

Status: Needs work » Needs review

@Amber Himes Matz: Thanks for the review! Normally stuff needs to go to that location (until help topics are no longer experimental). However, this is a special case since it's a deprecated module that's being removed from core. @xjm said that deprecated QE is more relevant than experimental HT, and that we're supposed to move the files. That already happened at #3264945: Move quickedit help topics to quickedit module.

This issue is only about fixing problems in the text that were introduced there, which I only really noticed when reviewing this to commit it to the contrib QE: #3264949: Move Quick Edit help topics to contrib Quick Edit module

Spokje’s picture

Sorry, but setting back to needs review until the file placement and is this issue filed against the correct branch question are answered.

@Amber Himes Matz in #12

File placement issue:

Ok, I applied the patch in #5 and it created core/modules/quickedit/help_topics/quickedit.quick_edit.html.twig.

This is the file that will exist in Drupal 10. But in Drupal 9, all core module help_topics should still be in core/modules/help_topics/help_topics.

@Amber Himes Matz in #12

I think the file placement issue has already been covered by @dww in #14.

Is this issue filed against the correct branch question:

At present this issue is labeled with version 9.4.x-dev. Is that an error?

@Amber Himes Matz in #12

I think/hope this quote from the IS of #3264949: Move Quick Edit help topics to contrib Quick Edit module will clear that up:

In #3264945: Move quickedit help topics to quickedit module the help topics related to the quickedit module are moved from the Core help_topics module to the Core quickedit module.

In Drupal 10 these help topics will be removed when the whole quickedit module will be removed.

So the quickedit help topics currently live in the quickedit module in both the 9.4.x and 10.0.x branch.

Since the whole quickedit module (and thus its help topics) will be removed from 10.0.x when #3227033: Remove Quick Edit from core lands we (or at least I) think that doing the changes in the issue against 10.0.x and backport to 9.4.x would be both a lot of effort for something that will be removed from 10.0.x shortly thereafter, and would even make it a blocker for #3227033: Remove Quick Edit from core.
Hence the reasoning to put this issue against 9.4.x only.

I hope I've (or basically @dww) explained it correctly and it all makes sense?

As far as I'm concerned this issue could go back to RTBC, but I hope that @Amber Himes Matz confirms the above ramblings are indeed an answer to both questions.

Amber Himes Matz’s picture

Status: Needs review » Reviewed & tested by the community

Ok, thank you both for the clarification! I didn't know about the deprecation policy with regard to help topics. This will certainly be relevant for a number of other core modules with help topics that are planned for removal in Drupal 10.

That was my only concern. Looks like the steps have been reviewed and approved, so marking this back to RTBC. Thank you all!

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 5: 3265492-4.patch, failed testing. View results

dww’s picture

Status: Needs work » Reviewed & tested by the community

Random fail

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 5: 3265492-4.patch, failed testing. View results

dww’s picture

Status: Needs work » Reviewed & tested by the community

More random fail. Thanks, layout_builder. 😉

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

  • catch committed cc39d49 on 10.0.x
    Issue #3265492 by dww, Spokje, Amber Himes Matz, quietone: Fix...
  • catch committed 8fbe9b5 on 9.4.x
    Issue #3265492 by dww, Spokje, Amber Himes Matz, quietone: Fix...
  • catch committed 8afa78a on 9.5.x
    Issue #3265492 by dww, Spokje, Amber Himes Matz, quietone: Fix...
catch’s picture

Version: 9.5.x-dev » 9.4.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 10.0.x, cherry-picked to 9.5.x and 9.4.x, thanks!

Status: Fixed » Closed (fixed)

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