Add a developer tutorial for displaying notices #13703
Conversation
danielbachhuber
requested review from
ajitbohra,
chrisvanpatten,
gziolo,
mkaz,
oandregal and
youknowriad
as code owners
There was a problem hiding this comment.
The question I have is where to put the JavaScript code? The tutorial should include a small part of enqueue and dependencies, it might even need to include a dependency beyond wp.data so it runs after the editor is instantiated.
If it is to be a tutorial it probably should be more explicit with all steps. I would also be fine with moving it as an example part of the data-core-notices.md but the tutorial is probably more useful since the same question on where to put the code would come up.
Couple of questions:
Also, as a point of process, is there a way of knowing what's required of a tutorial before I submit it as a pull request? There's been a Paper doc sitting in #13592 for a week that I never received any feedback on. |
docs/designers-developers/developers/tutorials/notices/README.md
Outdated
Show resolved
Hide resolved
|
I agree w/ Daniel that it will get duplicative if we keep including enqueue code in every tutorial. I think it's fine to add a note along the lines of "Make sure you have a JavaScript file available and enqueued" and link to the appropriate page in the JS tutorial. @danielbachhuber In terms of process, we've been starting from PRs rather than working in other contexts. If you have something that you'd specifically like reviewed before getting a PR up I'd almost even prefer that you just open a PR that maybe adds the blank markdown file and links to the doc in the contents of the PR text, so we can consolidate as much discussion as possible in PRs. EDIT: That is to say… I see PRs as the place for revision and collaboration on a specific/extent piece of proposed content or code. |
Fair enough, although I didn't know where to put the document originally. |
Co-Authored-By: danielbachhuber <daniel@bachhuber.co>
docs/designers-developers/developers/tutorials/notices/README.md
Outdated
Show resolved
Hide resolved
| * Hook into the 'admin_notices' action to render | ||
| * a generic HTML notice. | ||
| */ | ||
| add_action( 'admin_notices', function() { |
There was a problem hiding this comment.
So personally I have no issues with using closures inside actions (in fact I love them myself and almost hate that I'm commenting about this)… but I think, at least until the day core officially drops 5.2, this should be written with a full function name and all that nonsense.
Sigh.
There was a problem hiding this comment.
There was a problem hiding this comment.
FINGERS ALL THE WAY CROSSED
There was a problem hiding this comment.
Changed to a function in 07e8a60
|
@danielbachhuber Totally fair. And it wasn't helped by my ongoing issue backlog. In the future hopefully I can be faster about feedback there. |
|
I'm fine with not duplicating the enqueue code for each tutorial, but I feel including a sentence that it should be included and a link to the JS tutorial would be sufficient. I think having a documentation labeled as a tutorial it should have all the steps necessary to get from point A to point B. It doesn't necessarily have to include every step, but should refer to spots that it is included. Otherwise, it could be just an example in the data-notices documentation and not as a tutorial. As far as process, this showed up in my "requested for review" and the other didn't - I think this is due to the new ownership process. |
|
I won't even try this one as a suggestion :-) In the documentation the link would be: |
Fair enough.
Added in d887d23 |
danielbachhuber
added
[Type] Enhancement
* Add a developer tutorial for displaying notices * Update `manifest.json` with new document * Fix link to available actions and selectors Co-Authored-By: danielbachhuber <daniel@bachhuber.co> * Avoid closures per feedback * Use 'core' to distinguish between WordPress, themes, and plugins * Link to the "How to JavaScript" tutorial for further detail
* Add a developer tutorial for displaying notices * Update `manifest.json` with new document * Fix link to available actions and selectors Co-Authored-By: danielbachhuber <daniel@bachhuber.co> * Avoid closures per feedback * Use 'core' to distinguish between WordPress, themes, and plugins * Link to the "How to JavaScript" tutorial for further detail
Fixes #13592