Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expand $args array parameters that are described in other functions #96

Merged
merged 7 commits into from
Jun 10, 2022

Conversation

tellyworth
Copy link
Contributor

Quick WIP version. See #95.

Before, on the get_terms() function reference:
Screen Shot 2022-06-08 at 4 13 40 pm

After, collapsed (detail/summary tag - note the teeny tiny triangle):
Screen Shot 2022-06-08 at 4 15 11 pm

After, expanded:
Screen Shot 2022-06-08 at 4 15 25 pm

Before, on get_posts(). Note that this one has a few arguments explicitly documented:
Screen Shot 2022-06-08 at 4 21 35 pm

After, collapsed:
Screen Shot 2022-06-08 at 4 21 51 pm

After, expanded (note that it still has the explicit docs first, I haven't hidden that):
Screen Shot 2022-06-08 at 4 22 28 pm

There are definitely some improvements needed especially on the front-end. But this seems to work pretty well at recursing through and finding the correct array. For example it works with multiple levels of indirection, like wp_count_terms($args) -> get_terms($args) -> WP_Term_Query::__construct($query).

@tellyworth tellyworth requested a review from dd32 June 8, 2022 06:27
@ryelle
Copy link
Contributor

ryelle commented Jun 8, 2022

I pushed up a commit to remove the summary wrapper from the description — that was stripping the semantics from everything inside it. It's best to just have the content in summary be a single text node (or heading).

I just changed it so it's "More Arguments". Feel free to change that to whatever works better :)

An example on get_posts:

Screen Shot 2022-06-08 at 4 26 53 PM

And on wp_count_terms:

Screen Shot 2022-06-08 at 4 27 03 PM

I'm also seeing <br> on all the params now, I think that's something new on this branch.

@tellyworth
Copy link
Contributor Author

Thanks!

I'm not seeing the <br> thing at all. Not sure what's up with that. Some kind of nl2br formatting, maybe on import? I re-ran an import and it didn't reproduce.

I pushed a couple of commits riffing on this a bit. The summary now looks more obviously clickable, and explains where the extra info came from:
Screen Shot 2022-06-09 at 5 12 44 pm

It's a bit better but still needs some work. Might be almost good enough to merge now for a designer to look at later - what do you think? Assuming I can fix the br thing.

Copy link
Contributor

@ryelle ryelle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not seeing the
thing at all. Not sure what's up with that. Some kind of nl2br formatting, maybe on import? I re-ran an import and it didn't reproduce.

🤷🏻 I'm not seeing them on trunk, just this branch. Ah okay — I just rebased and they're gone now. I'm not sure why you weren't seeing them but when this merges it should be fine, at least.

Might be almost good enough to merge now for a designer to look at later - what do you think?

That works for me. I don't think it should look like a link (since it's not taking you anywhere), but we can iterate on that later.

@tellyworth tellyworth added this to the Docs Sprint milestone Jun 10, 2022
@tellyworth tellyworth force-pushed the add/expand-recursive-parameters branch from 3ba4dda to 60db90e Compare June 10, 2022 04:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants