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

Implement repository configuration with asciidoctor-tabs #3104

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ekohl
Copy link
Member

@ekohl ekohl commented Jul 2, 2024

Rather than two procedures below eachother, this implements tabs. That makes the overall page shorter while still obvious.

The implementation uses asciidoctor-tabs. The diff looks huge because it inserts CSS into every page.

Currently very much a proof of concept. It's only implemented in the server installation guide and it probably breaks downstream. Please consider this as more of a discussion starter.

image

  • I am okay with my commits getting squashed when you merge this PR.
  • I am familiar with the contributing guidelines.

Please cherry-pick my commits into:

  • Foreman 3.11/Katello 4.13
  • Foreman 3.10/Katello 4.12
  • Foreman 3.9/Katello 4.11 (Satellite 6.15; orcharhino 6.8)
  • Foreman 3.8/Katello 4.10
  • Foreman 3.7/Katello 4.9 (Satellite 6.14)
  • Foreman 3.6/Katello 4.8
  • Foreman 3.5/Katello 4.7 (Satellite 6.13; orcharhino 6.6/6.7)
  • We do not accept PRs for Foreman older than 3.5.

Copy link

github-actions bot commented Jul 2, 2024

The PR preview for 4fea8e0 is available at theforeman-foreman-documentation-preview-pr-3104.surge.sh

The following output files are affected by this PR:

show diff

show diff as HTML

@ekohl
Copy link
Member Author

ekohl commented Jul 2, 2024

Another use case: you can implement all 3 procedures in different tabs. One tab for the UI, one for CLI and another for Ansible. asciidoctor-tabs can also be configured to remember the selection, so users can benefit from always viewing their preferred way.

@Lennonka
Copy link
Contributor

Lennonka commented Jul 2, 2024

It would be so awesome, but I would advise against using extensions when we need to convert it to DITA for Satellite downstream. This would add unnecessary complexity for the conversion.

@asteflova
Copy link
Contributor

At first glance, I love it! On second thought, it makes full text searches more difficult. But only for users whose tabs are switched to, for example, RHEL 8, but would appreciate seeing search results for RHEL 9. Whether this is an issue or not depends on how often users would need to search for information that's outside of their current selection.

@apinnick
Copy link
Contributor

apinnick commented Jul 4, 2024

Unfortunately, this Asciidoctor function is not supported downstream. If I recall correctly, the blocker was Pantheon, which could not render the tabs. Maybe things will be different when we change our tooling.

@ekohl
Copy link
Member Author

ekohl commented Jul 4, 2024

Unfortunately, this Asciidoctor function is not supported downstream. If I recall correctly, the blocker was Pantheon, which could not render the tabs. Maybe things will be different when we change our tooling.

In an internal doc I see quarkus-documentation has some tabs implementation where they do replacements to convert from tabs to non-tabs. Looking at https://github.com/quarkusio/quarkus/tree/main/docs/src/main/asciidoc they have their own implementation (https://github.com/quarkusio/quarkus/blob/main/docs/src/main/asciidoc/javascript/asciidoc-tabs.js).

@ekohl
Copy link
Member Author

ekohl commented Jul 4, 2024

Also, if you run asciidoc without the asciidoc-tabs plugin it just ignores the tabs part and renders like this:
Screen Shot 2024-07-04 at 11 47 34
The headers may not be great because they're intended for tabs, but perhaps downstream can do something similar?

@apinnick
Copy link
Contributor

apinnick commented Jul 4, 2024

@ekohl If you are saying that this would be a nice thing to have in Foreman even if we cannot do it d/s, I agree.

However, we would still have to do some scripting because Pantheon does not use plain asciidoc to build the guides. I'm not saying this is a blocker, just that the file would still require some processing.

@asteflova
Copy link
Contributor

Is using conditionals an option? Use tabs for upstream ifdefs only and stick to the existing headings and other markup for d/s?

@ekohl
Copy link
Member Author

ekohl commented Jul 4, 2024

I considered the same thing. It may be a nightmare to maintain, it may be doable. If you have ideas how we can explore that then I'm all ears

@asteflova
Copy link
Contributor

If we stick to using tabs with web UI/CLI/API/Ansible only, the maintenance nightmare might not be so bad. We could write a template to make creating the procedures easier..? And I wouldn't be so concerned with full text searching if we only implement it for these use cases.

In any case, I think it's a good topic for a community demo: Present the concept (to collect feedback on whether this is even a good idea) along with a few possible ways to implement it (scripting pain to accommodate d/s vs. lovecraftian conditionals).

@ekohl
Copy link
Member Author

ekohl commented Jul 4, 2024

If we stick to using tabs with web UI/CLI/API/Ansible only, the maintenance nightmare might not be so bad. We could write a template to make creating the procedures easier..? And I wouldn't be so concerned with full text searching if we only implement it for these use cases.

I agree we should limit its use. I think repository configuration (as I did here) is a prime example because full text search is largely irrelevant. Splitting up procedures may also be good, but I here others are probably more experienced to say something useful.

In any case, I think it's a good topic for a community demo: Present the concept (to collect feedback on whether this is even a good idea) along with a few possible ways to implement it (scripting pain to accommodate d/s vs. lovecraftian conditionals).

I didn't consider the community demo yet, but do agree that you want to experiment with some implementations to see what feels like the least bad hack.

Should we first discuss in our docs meeting before we bring it to a community demo?

@asteflova
Copy link
Contributor

It occurred to me that using tabs for CLI/web UI/whatever procedures might actually turn out to be a really bad idea. Complete symmetry would be needed between these management interfaces. However, in some situations, there is no feature parity between them; in others, it might simply not be productive to describe each action using each interface.

@Lennonka
Copy link
Contributor

Lennonka commented Jul 5, 2024

I think that instead of starting another thing we're not going to finish, we should focus on seeing through the stuff that is already in progress.
Like moving away from the manual to docs for Foreman and making guides usable in el and deb builds.

@ekohl
Copy link
Member Author

ekohl commented Jul 11, 2024

This was discussed during the docs meeting today and the consensus was that it's a good idea. There may be some implementation details, but we can start small. Initially focus on upstream-only parts. Things like setting up repositories is an upstream-only aspect. Restructuring modules to be upstream-only may be a good idea.

@ekohl
Copy link
Member Author

ekohl commented Jul 11, 2024

I've changed the implementation. It now first refactors the single file to multiple, more focused, files. Then there's a commit that adds tabs only to the upstream files. I'm not as familiar with best practices around modules, so I'd appreciate feedback on the first commit. That should also show it's effectively a noop in the final result. At least, that was my intention with it. I've opened #3123 to verify that and it may be easier to leave focused reviews there and leave this PR to just the tabs implementation.

For now I only tested this on the Installing server guide.

Like moving away from the manual to docs for Foreman and making guides usable in el and deb builds.

I actually see this as part of that effort. In the current manual we have a selector and if we add tabs, we don't regress in that aspect.

Rather than two procedures below eachother, this implements tabs. That
makes the overall page shorter while still obvious.
@ekohl ekohl marked this pull request as ready for review August 17, 2024 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants