asyncapi / website

AsyncAPI specification website
https://www.asyncapi.com
Apache License 2.0
422 stars 569 forks source link

[Docs Bug 🐞 report]: Automate the Prev and UpNext buttons for Docs #1000

Closed akshatnema closed 1 year ago

akshatnema commented 1 year ago

Describe the bug you found in AsyncAPI Docs.

Every time we update the docs or add a new doc inside documentation, we have to add Up Next and Back buttons for each page or update them to the latest. Make the buttons in such a way that it automates itself to point to next and back pages sequentially according to the Doc tree.

Remove the DocsButton component from each Doc page and add the component to the DocsLayout, and implement it according to the index of the map function used.

Attach any resources that can help us understand the issue.

image

Use the weight parameter to serialize the docs and then provide the routes to the buttons for each page.

Code of Conduct

toukirkhan commented 1 year ago

So the upnext in https://www.asyncapi.com/docs/tutorials/streetlights-interactive should be linked to https://www.asyncapi.com/docs/tutorials/streetlights ?

manavdesai27 commented 1 year ago

Hey can I contribute to this issue? @akshatnema

akshatnema commented 1 year ago

So the upnext in asyncapi.com/docs/tutorials/streetlights-interactive should be linked to asyncapi.com/docs/tutorials/streetlights ?

There are many changes related to this and also regarding the overview pages for each subtopic. All of them will be covered in this issue.

Hey can I contribute to this issue? @akshatnema

Surely @manavdesai27, you can contribute to the issue. Let us first have approval from @derberg and @alequetzalli.

toukirkhan commented 1 year ago

basically what I can observe over here is that we need to provide buttons in the overview section and correctly the buttons in every page. Right?

quetzalliwrites commented 1 year ago

Hey @akshatnema this is actually 3 different bugs listed under one issue... Can you at least create 3 tasks in this same issue to help make it more digestible? 😜🙌🏽 I also recommend thinking of a new title that better encapsulates the 3 items you discuss. 👍🏽👍🏽

As for adding Prev/Next buttons on each content bucket /Overview pages, I think that's a great idea 💡 . I am ok with a contributor adding them and I assume that Lukasz prob is too. (altho yes, I should never assume 😂)

akshatnema commented 1 year ago

basically what I can observe over here is that we need to provide buttons in the overview section and correctly the buttons in every page. Right?

Yes, exactly, but we have to make sure that whether it is needed on the Overview page or if we should have another way of cards/buttons to link all pages under it.

Hey @akshatnema this is actually 3 different bugs listed under one issue... Can you at least create 3 tasks in this same issue to help make it more digestible? 😜🙌🏽

Hmm, if I look into the issue, I don't think it can be converted into the more digestible issues, because with each page, there is only 1 line page and making a different issue for each line page, I won't prefer that. Rest I will like to have an opinion with @derberg. Secondly, I haven't mentioned all the link changes in this issue because it is self-defined from the tree diagram inside the docs side bar how the pages should be linked with each other. So, I will like to make sure that we don't have spam of PRs with just a one-two liner change in a page. Better I would go with code-related contribution with this as we already have a component designed for these buttons. You can probably look on this line: https://github.com/asyncapi/website/blob/master/pages/docs/tutorials/streetlights.md?plain=1#L257

toukirkhan commented 1 year ago

@manavdesai27 working on this one?

manavdesai27 commented 1 year ago

@manavdesai27 working on this one?

Yes, I'm working on this issue.

toukirkhan commented 1 year ago

@manavdesai27 okay you take this one.

manavdesai27 commented 1 year ago

Where should I point the UP Next button of Reference Overview to, or should I even keep an Up Next button for Reference? Currently I am pointing it to Specification v2.5.0as Specification page has a url for the same. @akshatnema

I am attaching the screenshot below for your reference:

Screenshot from 2022-10-05 10-36-35

magicmatatjahu commented 1 year ago

Take into account that bugs will sooner or later appear again, because the more docs will arrive, so I would advise to go in the direction of automatically checking the prev/next pages and generate prev/next button based on that. I don't remember exactly if we already have the necessary metadata during rendering, but we can always generate it at generation step from docs mdx to JSONs.

akshatnema commented 1 year ago

I would advise to go in the direction of automatically checking the prev/next pages and generate prev/next button based on that.

@magicmatatjahu We don't have any predefined sequence or tree specified in the codebase. So, how can we automate the prev and next buttons according to the docs tree specified right now?

derberg commented 1 year ago

Still nextjs haven't made this an error because it redirects it to /docs/tutorials/getting-started/event-driven-architecture (I don't know how 😅).

@akshatnema during the restructuring of docs @alequetzalli also handled netlify redirects configuration for old paths, to make sure they still work. This is why it works even if path is wrong 😄

We don't have any predefined sequence or tree specified in the codebase. So, how can we automate the prev and next buttons according to the docs tree specified right now?

we have weight metadata, so next after the document with weight: 40 should be the document with weight > 40, and in case there is no such, take the first document with the smallest weight from next folder... 🤔 What is the next folder? maybe something we need to specify in _section.md file?

As for adding Prev/Next buttons on each content bucket /Overview pages, I think that's a great idea 💡 . I am ok with a contributor adding them and I assume that Lukasz prob is too. (altho yes, I should never assume 😂)

very good assumption @alequetzalli 😄

Hey @akshatnema this is actually 3 different bugs listed under one issue... Can you at least create 3 tasks in this same issue to help make it more digestible? 😜🙌🏽 I also recommend thinking of a new title that better encapsulates the 3 items you discuss. 👍🏽👍🏽

yeah, we have a bit of mix here. I think we agree that best solution is to automate. This is not as trivial as just first fixing these next/prev buttons manually.

I recommend this issue stays for automation related implementation and discussion. And you open a separate issue where you agree with @alequetzalli what buttons, with what values have to be added to what documents, and some hacktoberfest participant can quickly fix that.

This way we will validate if making it automated using weight actually makes sense

akshatnema commented 1 year ago

@akshatnema during the restructuring of docs @alequetzalli also handled netlify redirects configuration for old paths, to make sure they still work. This is why it works even if path is wrong 😄

But tbh, it's not a good way right now. Because if you try to access the page in development mode, it broke down giving a 404 page as redirection is not set there.

I recommend this issue stays for automation related implementation and discussion. And you open a separate issue where you agree with @alequetzalli what buttons, with what values have to be added to what documents, and some hacktoberfest participant can quickly fix that.

Ok, let's make this issue specific to automation where we will try to make these buttons understand the Up Next and Previous links for a specific page. I create another issue for these small changes and give it a hacktoberfest tag. Meanwhile, we can remove that tag from here, because we still need some discussion here on how to automate it 😅.

derberg commented 1 year ago

But tbh, it's not a good way right now. Because if you try to access the page in development mode, it broke down giving a 404 page as redirection is not set there.

well, I didn't say it must stay like this 😄 redirects were done for people linking to AsyncAPI website from outside. I just explained why it works 😄 this link should definitely be fixed in the markdown file.

I create another issue for these small changes and give it a hacktoberfest tag.

@manavdesai27 is super motivated to do both issue, so let us leave hacktoberfest tag. There are still 2 weeks to go.

manavdesai27 commented 1 year ago

@manavdesai27 is super motivated to do both issue, so let us leave hacktoberfest tag. There are still 2 weeks to go.

Yeah sure, I will first work on issue #1014 . And then come back to this. That issue won't take much time to fix. @derberg

akshatnema commented 1 year ago

@manavdesai27 @alequetzalli I have updated the issue description according to the need. Kindly look into it.

derberg commented 1 year ago

hey folks, IMHO we need 2 mechanisms:

wdyt?

toukirkhan commented 1 year ago

Both options LGTM

quetzalliwrites commented 1 year ago
  • one proposed by @akshatnema based on weight. So automatically detect what is next, and what previous

📢📢📢 YES, I love this one, this is a good idea.

  • but ☝🏼 should be "fallback" and main mechanism should be a front matter, that docs author can add to markdown file, and specify next and previous that is actually different than weights

wait what? sorry, I feel confused and don't think I understand this point.. can I bother you to explain it more to me, @derberg ? 😬😂😂

derberg commented 1 year ago

It basically means that by default it is based on weight but if someone added:

title: My cool doc
weight: 10
nav_buttons:
    next: /docs/concepts/channel
    previous: /docs/concepts/application

then values for buttons will be taken from above front matter. how about now 😅

akshatnema commented 1 year ago

image

@derberg @alequetzalli what if we make an object of posts copied/calculated using the DocsNav tree object, add prev and next fields using previous and next elements respectively, and then pass it as post to the component?

derberg commented 1 year ago

in the end this tree is build upon the weight right? yes, just also as I wrote, content providers should have an option to provide front matter that will override default behaviour.

akshatnema commented 1 year ago

in the end this tree is build upon the weight right?

Nope, that's the most surprising thing I got today. Docs are not entirely sorted using weights. We have not provided any global or sequential parameter to sort the doc. So, the tree is made using appending of children inside each root section and subsection.

derberg commented 1 year ago

have a look at https://github.com/asyncapi/website/blob/master/components/layout/DocsLayout.js#L26 where we build the tree. We are sorting by few properties, weight including. Children that you are referring to is just the way how these subsections (like guides or generator) are added to the tree, but these are identified by https://github.com/asyncapi/website/blob/master/scripts/build-post-list.js#L27 where we walk through all directories, identify subdirectories as not root sections, etc.

in the end, navigation is build from weights. And yes, it doesn't make sense to write separate new logic to identify next/prev basing on weight if we already have it in the navigation tree, so just reuse navigation from const navigation = buildNavTree(navItems);

akshatnema commented 1 year ago

image

@derberg Look at this image carefully. This is the data inside the navigation only. If we look deeply, it is not sorted properly. welcome is assigned as the last element whereas it is the first one. It should follow the order welcome -> concepts -> tutorials -> tools -> guides -> references. Even if I expand each element here, the children (doc pages inside it) are not sorted sequentially according to the weight.

Preview of concepts object: image

This is not exactly according to the weight sorted docs.

akshatnema commented 1 year ago

image

Although 2 different log messages of same object 😅

derberg commented 1 year ago

the first image is console log from browser? maybe by default it orders object keys alphabetically in the browser dev tools?

akshatnema commented 1 year ago

the first image is console log from browser? maybe by default it orders object keys alphabetically in the browser dev tools?

Yeah maybe, not sure about it, and also, we don't have to do the computation stuff regarding the addition of prev and next fields in the DocsLayout.js file. Instead, it should be in the build-post-list.js file to add the fields in all posts at the time of compilation of posts. So, probably, we will be copying this logic to that file to get the sorted list of tools in the posts.json or we can do the same stuff of logic in Layout.js file.

derberg commented 1 year ago

Sure, just keep in mind that can be done as separate refactor task for other contributors

quetzalliwrites commented 1 year ago

This appears to be a common question in the internet 😄 .. lots of blog posts and StackOverflow threads mention it

Perhaps this can help?

The iteration order for objects follows a certain set of rules since ES2015, but it does not (always) follow the insertion order. Source: https://stackoverflow.com/questions/5525795/does-javascript-guarantee-object-property-order