Telling developers that %variable will make text into an EM element and has a "placeholder" class on it.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Pancho’s picture

Priority: Normal » Minor

I'm not sure that's necessary. Did you encounter an error?
Our docs are parsed by IDEs and tools that don't know our "placeholder" class. Also there's no precedence in core.

droplet’s picture

hmm. When developers read the docs, they don't know it added placeholder class to drupal_placeholder. Is it an error ??

Our docs are parsed by IDEs and tools that don't know our "placeholder" class.

Sorry, I don't understand this point ??

jhodgdon’s picture

Status: Needs review » Postponed (maintainer needs more info)

What are you trying to do here? If you want to say that the text is made into an em HTML tag and the class on that tag is "placeholder", then this is not the right way to do it -- you should state that in plain English in the API documentation instead. If you are trying to do something else, then... what?

droplet’s picture

Status: Postponed (maintainer needs more info) » Needs work
Issue tags: +Novice

If you want to say that the text is made into an em HTML tag and the class on that tag is "placeholder", then this is not the right way to do it

YES.

OK, anybody can rewrite it into plain English or whatever meet Drupal Standard. Thanks.

Pancho’s picture

Issue tags: -Novice

@droplet: Oh, this has not been your fault.
I thought you wanted it to be rendered with that class, and just didn't get why you would want that... :)

But yeah, it certainly makes sense to document the class that is being added. The question is, if we can use (unescaped) HTML tags in docs. For example PHPDoc parses <b> or <i> in HTML but leaves <strong> or <em> alone, so according to their docs it would actually work. Drupal's API module doesn't parse anything in HTML, so it work there anyway. IDE's might have a different take on it.

To be on the secure side, we should probably avoid it, but:
- firstly it was already there (you added just the class)
- secondly our Docs Styleguide doesn't mention anything about it.

Quite more tricky than a Novice issue.
We might even want to escalate this, so we get a clear decision we can add to the styleguide.

jhodgdon’s picture

Please just make a plain English text describing what happens, like "makes an HTML em tag with class foo" and you can use @code @endcode around the tag to get it to show up as <em> on api.drupal.org if you want. The other entries in this help text could be fixed up too.

jhodgdon’s picture

Issue summary: View changes

Updated issue summary.

er.pushpinderrana’s picture

Issue summary: View changes
Status: Needs work » Needs review
FileSize
934 bytes
935 bytes

Added @code @endcode around the tag to get it to show up as on api.drupal.org if you want. Please review.

jhodgdon’s picture

Status: Needs review » Needs work

Thanks... But I do not think that the text makes sense if you use @code/@endcode to show exactly what HTML markup is being used. The text should say something like "... which makes the following HTML code:", and I don't think making the text inside the EM tag say "emphasized" actually makes any sense at all for the sample -- put something like "text output here" or some other more sensible placeholder.

    *   - %variable: Escaped to HTML and formatted using String::placeholder(),
-   *     which makes it display as <em>emphasized</em> text.
+   *     which makes it display as
+   *     @code <em class="placeholder">emphasized</em> @endcode text.

er.pushpinderrana’s picture

Status: Needs work » Needs review
FileSize
969 bytes
1015 bytes

@jhodgdon thanks!

Please review this one.

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

Much better, thanks!

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 48eabc0 and pushed to 8.x. Thanks!

jhodgdon’s picture

Status: Fixed » Reviewed & tested by the community

The link to your commit didn't work and the auto-commit message didn't show up. Are you sure that was committed? I'm not seeing the change in the repo...

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

  • alexpott committed 48eabc0 on 8.x
    Issue #2036611 by er.pushpinderrana, droplet: Fix drupal_placeholder doc...

Status: Fixed » Closed (fixed)

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