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

Blocks to show parents in docs headers. #5381

Closed
wants to merge 1 commit into from

Conversation

ptodev
Copy link
Contributor

@ptodev ptodev commented Oct 5, 2023

There are two issues with the current best practices for writing flow components:

  1. They do not say what to do in case two blocks have the same name.
  2. It is hard to tell where each block fits in the config hierarchy just by looking at the table of contents on the right of the webpage

#5229 introduced the otelcol.processor.k8sattributes component which contains two blocks called "label". The best practices for writing flow components docs do not mention what to do in this case. If we follow them, we will end up with two blocks with the same name in the table of contents:

Screenshot 2023-10-05 at 18 14 21

In this PR, I am proposing two changes:

  • Including the parents of a block in the header. This makes it more clear which block this heading is referring to.
  • Dropping the "block" word from the header. Every header under "Blocks" is a block, so having the word "block" in each heading is redundant.

This is what the table of contents looks like for otelcol.processor.k8sattributes after this change:

Screenshot 2023-10-05 at 18 14 03

I believe this looks much cleaner. There are only two issues which I can think of:

  1. Obviously, it's a lot of work to update all the docs to follow this convention. For now we can just do it for otelcol.processor.k8sattributes, and we can also update other docs over time. I believe the long term gain in usability is worth the short term inconsistency.

  2. The table of contents might look a bit off for deeply nested blocks. The name of the block goes on another line, and that line starts to look like a whole other block hierarchy. However, I think this is not such a bit problem, because when you mouse over you see that it's all one big link. Also, I don't expect deeply nested blocks to be common (but I have not checked if that's the case). It might be better if we somehow don't let an overflow to a new line though, and instead we could have a horizontal scrollbar like the one we have if a block name is too long (see point 3). I could raise a ticket with the website team to ask if it's possible?
    Screenshot 2023-10-05 at 18 34 26
    Screenshot 2023-10-05 at 18 34 34

  3. I also tried how it looks for blocks with very long names - we get a scrollbar, so I think that's all good:

Screenshot 2023-10-05 at 18 35 06

@ptodev ptodev requested review from clayton-cornell and a team as code owners October 5, 2023 17:42
@rfratto
Copy link
Member

rfratto commented Oct 5, 2023

I'm worried about the side effects of introducing content duplication on the same page for components where the same block is used in multiple parts of the overall hierarchy. In particular, I'm concerned that readers will get overwhelmed with thinking that there's more behaviors about a component to learn, when in reality some behavior is shared.

Aside from the hierarchy table we have today, are there any other ways we can include metadata about a particular block to show it's hierarchy that don't involve literally including it on the page multiple times?

@ptodev
Copy link
Contributor Author

ptodev commented Oct 6, 2023

Hi @rfratto, I think you are referring to a different issue. The goal of this PR is to make distinct headers for two blocks which have the same name. In the case of otelcol.processor.k8sattributes there is both an "extract > label" block and a "filter > extract" block. Those two blocks contain a distinct set of attributes.

I agree with you about the repetition, but I think it should be handled in a separate PR. In the case with otelcol.processor.k8sattributes, there is repetition in:

  • filter > field and filter > label
  • extract > annotation and extract > label

@clayton-cornell clayton-cornell added the type/docs Docs Squad label across all Grafana Labs repos label Oct 6, 2023
@clayton-cornell
Copy link
Contributor

clayton-cornell commented Oct 13, 2023

I haven't checked yet... but... how much deeper do we go than:

filter > field
filter > label

and so on? Are there cases of field > label > something > something else ?

Another option would be to make these another heading level and H4 should still work cleanly (I want to test it) So the toc would be more like:

Blocks
...
    filter
        field
        label
    pod_association
    exclude
        pod
    output
Exported fields

@clayton-cornell
Copy link
Contributor

@ptodev Just thought of something on this that might make the proposed change a bit awkward. When you have child elements used in more than one place like...

  • extract > label
  • filter > label
    you will have the label child block documented 2x (or more) within in the topic.

@ptodev
Copy link
Contributor Author

ptodev commented Dec 7, 2023

A much broader overhaul of how blocks are documented is underway. It will include parts of this PR. I am closing this PR so that we can open a new one with much more changes.

@ptodev ptodev closed this Dec 7, 2023
@ptodev ptodev deleted the ptodev/duplicate-headings branch December 7, 2023 12:17
@github-actions github-actions bot added the frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed. label Feb 21, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed. type/docs Docs Squad label across all Grafana Labs repos
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants