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

Docs - render object choices #4442

Merged
merged 5 commits into from
Nov 5, 2023
Merged

Docs - render object choices #4442

merged 5 commits into from
Nov 5, 2023

Conversation

himdel
Copy link
Collaborator

@himdel himdel commented Oct 25, 2023

ansible/ansible#81951 (comment)

In docs renderer, choices used to be an array of string values, that's still permissible,
but now we also accept an object of { value: description } instead of the array.

This also changes the rendering of choices to be closer to https://docs.ansible.com/ansible/devel/collections/ansible/builtin/config_lookup.html#parameter-on_missing
=> monospace the choice value, "value: description"

Before - Array:

20231025011837

Before - Object:

20231101021437

Now - Array:

20231101021542

Now - Object:

20231101020734
20231101020747

(also fixed the error message...

20231101022139
)

@github-actions github-actions bot added backport-4.7 This PR should be backported to stable-4.7 (2.4) backport-4.8 This PR should be backported to stable-4.8 (2.4) labels Oct 25, 2023
{legends[c] ? (
<>
{' - '}
{legends[c]}
Copy link
Contributor

Choose a reason for hiding this comment

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

legends[c] can be either a string or a list of strings (if it's something else it is broken), and it can contain Ansible markup.

I would probably normalize it to arrays in parseOptions, and do something like

            {legends[c].map((d) => (
              <p>{this.applyDocFormatters(d)}</p>
            ))}

with it. Or maybe (to avoid strange formatting) don't add <p>, and replace </p> by <br>? 🤷

previously, `choices` was a list of options,
now it can also be an object of options: description,
where description can be a string or an array of strings

No-Issue
@himdel himdel marked this pull request as ready for review November 1, 2023 02:13
@himdel
Copy link
Collaborator Author

himdel commented Nov 1, 2023

@felixfontein can you re-check please?

20231101020747

I updated choices to monospace the value, and show the potentially multiline (& formatted) description after.

Copy link
Contributor

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

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

Besides that, LGTM!

src/components/render-plugin-doc/render-plugin-doc.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

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

I haven't tested this and I'm not that familiar with React, but the way I understand the code it looks good :)

Copy link
Contributor

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

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

(Should have been an 'approve' review :D )

@himdel himdel merged commit 269af3f into ansible:master Nov 5, 2023
16 of 17 checks passed
@himdel himdel deleted the docs-choices branch November 5, 2023 00:51

This comment was marked as outdated.

Copy link

patchback bot commented Nov 5, 2023

Backport to stable-4.8: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-4.8/269af3fafcffdf0cb6551dc62f904951331cf766/pr-4442

Backported as #4489

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Nov 5, 2023
* RenderPluginDoc - support `choices` as an object

previously, `choices` was a list of options,
now it can also be an object of options: description,
where description can be a string or an array of strings

No-Issue

* PAUSED: Use `git resume` to continue working. [skip ci]

* Choice, Legend - render a monospace choice name, and a potentially multiline description

* also fix docs error rendering

* Update src/components/render-plugin-doc/render-plugin-doc.tsx

Co-authored-by: Felix Fontein <[email protected]>

---------

Co-authored-by: Felix Fontein <[email protected]>
(cherry picked from commit 269af3f)
himdel added a commit to himdel/ansible-hub-ui that referenced this pull request Nov 5, 2023
* RenderPluginDoc - support `choices` as an object

previously, `choices` was a list of options,
now it can also be an object of options: description,
where description can be a string or an array of strings

No-Issue

* PAUSED: Use `git resume` to continue working. [skip ci]

* Choice, Legend - render a monospace choice name, and a potentially multiline description

* also fix docs error rendering

* Update src/components/render-plugin-doc/render-plugin-doc.tsx

Co-authored-by: Felix Fontein <[email protected]>

---------

Co-authored-by: Felix Fontein <[email protected]>
(cherry picked from commit 269af3f)
himdel added a commit that referenced this pull request Nov 6, 2023
* RenderPluginDoc - support `choices` as an object

previously, `choices` was a list of options,
now it can also be an object of options: description,
where description can be a string or an array of strings

No-Issue

* PAUSED: Use `git resume` to continue working. [skip ci]

* Choice, Legend - render a monospace choice name, and a potentially multiline description

* also fix docs error rendering

* Update src/components/render-plugin-doc/render-plugin-doc.tsx

Co-authored-by: Felix Fontein <[email protected]>

---------

Co-authored-by: Felix Fontein <[email protected]>
(cherry picked from commit 269af3f)
himdel added a commit that referenced this pull request Nov 6, 2023
* RenderPluginDoc - support `choices` as an object

previously, `choices` was a list of options,
now it can also be an object of options: description,
where description can be a string or an array of strings

No-Issue

* PAUSED: Use `git resume` to continue working. [skip ci]

* Choice, Legend - render a monospace choice name, and a potentially multiline description

* also fix docs error rendering

* Update src/components/render-plugin-doc/render-plugin-doc.tsx

Co-authored-by: Felix Fontein <[email protected]>

---------

Co-authored-by: Felix Fontein <[email protected]>
(cherry picked from commit 269af3f)

Co-authored-by: Martin Hradil <[email protected]>
@himdel himdel added backported-4.7 This PR has been backported to stable-4.7 (2.4) backported-4.8 This PR has been backported to stable-4.8 (2.4) labels Dec 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-4.7 This PR should be backported to stable-4.7 (2.4) backport-4.8 This PR should be backported to stable-4.8 (2.4) backported-4.7 This PR has been backported to stable-4.7 (2.4) backported-4.8 This PR has been backported to stable-4.8 (2.4)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants