Add a developer tutorial for displaying notices #13703

danielbachhuber · 2019-02-06T18:45:32Z

mkaz

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.

danielbachhuber · 2019-02-06T23:53:12Z

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.

Couple of questions:

Is it necessary to have this for each tutorial, or could each tutorial link to a common reference point to avoid duplication?
Can you suggest what text I might use, to avoid back and forth? I don't have a strong preference.

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

chrisvanpatten · 2019-02-06T23:59:11Z

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.

danielbachhuber · 2019-02-07T00:01:33Z

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.

Fair enough, although I didn't know where to put the document originally.

Co-Authored-By: danielbachhuber <[email protected]>

chrisvanpatten

I only have two hopefully small changes. I think this is otherwise fine to land without any major changes. Would be great to expand in the future but obviously that requires the Notices API to expand too, which I'm excited for it to do :)

docs/designers-developers/developers/tutorials/notices/README.md

chrisvanpatten · 2019-02-07T00:03:04Z

docs/designers-developers/developers/tutorials/notices/README.md

+ * Hook into the 'admin_notices' action to render
+ * a generic HTML notice.
+ */
+add_action( 'admin_notices', function() {


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.

April 2019 per https://make.wordpress.org/core/2018/12/08/updating-the-minimum-php-version/

FINGERS ALL THE WAY CROSSED

Changed to a function in 07e8a60

chrisvanpatten · 2019-02-07T00:05:49Z

@danielbachhuber Totally fair. And it wasn't helped by my ongoing issue backlog. In the future hopefully I can be faster about feedback there.

mkaz · 2019-02-07T00:06:24Z

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.

mkaz · 2019-02-07T00:09:58Z

I won't even try this one as a suggestion :-)
Here's a potential for the JavaScript Tutorial link: https://wordpress.org/gutenberg/handbook/designers-developers/developers/tutorials/javascript/loading-javascript/

In the documentation the link would be: /docs/designers-developers/developers/tutorials/javascript/loading-javascript.md

danielbachhuber · 2019-02-07T00:17:22Z

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.

Fair enough.

Here's a potential for the JavaScript Tutorial link: https://wordpress.org/gutenberg/handbook/designers-developers/developers/tutorials/javascript/loading-javascript/

Added in d887d23

chrisvanpatten

I dig. 🚢

Change was accommodated 👍

* Add a developer tutorial for displaying notices * Update `manifest.json` with new document * Fix link to available actions and selectors Co-Authored-By: danielbachhuber <[email protected]> * 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

f15b1d9

danielbachhuber requested review from ajitbohra, chrisvanpatten, gziolo, mkaz, oandregal and youknowriad as code owners February 6, 2019 18:45

Update manifest.json with new document

bfccc4f

mkaz previously requested changes Feb 6, 2019

View reviewed changes

mkaz reviewed Feb 6, 2019

View reviewed changes

docs/designers-developers/developers/tutorials/notices/README.md Outdated Show resolved Hide resolved

Fix link to available actions and selectors

4f65dca

Co-Authored-By: danielbachhuber <[email protected]>

chrisvanpatten suggested changes Feb 7, 2019

View reviewed changes

danielbachhuber added 3 commits February 6, 2019 16:14

Avoid closures per feedback

07e8a60

Use 'core' to distinguish between WordPress, themes, and plugins

969ca24

Link to the "How to JavaScript" tutorial for further detail

d887d23

chrisvanpatten approved these changes Feb 7, 2019

View reviewed changes

danielbachhuber added [Type] Enhancement A suggestion for improvement. [Type] Developer Documentation Documentation for developers labels Feb 7, 2019

danielbachhuber added this to the 5.1 (Gutenberg) milestone Feb 7, 2019

danielbachhuber merged commit c763c03 into master Feb 7, 2019

danielbachhuber deleted the 13592-notices-tutorial branch February 7, 2019 01:24

noisysocks mentioned this pull request Feb 18, 2019

Bump plugin version to 5.1.0-rc.1 #13908

Merged

noisysocks mentioned this pull request Feb 20, 2019

Bump plugin version to 5.1.0 #13960

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a developer tutorial for displaying notices #13703

Add a developer tutorial for displaying notices #13703

danielbachhuber commented Feb 6, 2019

mkaz left a comment

danielbachhuber commented Feb 6, 2019

chrisvanpatten commented Feb 6, 2019 •

edited

Loading

danielbachhuber commented Feb 7, 2019

chrisvanpatten left a comment

chrisvanpatten Feb 7, 2019

mkaz Feb 7, 2019

chrisvanpatten Feb 7, 2019

danielbachhuber Feb 7, 2019

chrisvanpatten commented Feb 7, 2019

mkaz commented Feb 7, 2019

mkaz commented Feb 7, 2019

danielbachhuber commented Feb 7, 2019

chrisvanpatten left a comment

Add a developer tutorial for displaying notices #13703

Add a developer tutorial for displaying notices #13703

Conversation

danielbachhuber commented Feb 6, 2019

mkaz left a comment

Choose a reason for hiding this comment

danielbachhuber commented Feb 6, 2019

chrisvanpatten commented Feb 6, 2019 • edited Loading

danielbachhuber commented Feb 7, 2019

chrisvanpatten left a comment

Choose a reason for hiding this comment

chrisvanpatten Feb 7, 2019

Choose a reason for hiding this comment

mkaz Feb 7, 2019

Choose a reason for hiding this comment

chrisvanpatten Feb 7, 2019

Choose a reason for hiding this comment

danielbachhuber Feb 7, 2019

Choose a reason for hiding this comment

chrisvanpatten commented Feb 7, 2019

mkaz commented Feb 7, 2019

mkaz commented Feb 7, 2019

danielbachhuber commented Feb 7, 2019

chrisvanpatten left a comment

Choose a reason for hiding this comment

chrisvanpatten commented Feb 6, 2019 •

edited

Loading