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 Bug 🐞 report]: Automate the Prev and UpNext buttons for Docs #1000

Closed
1 task done
akshatnema opened this issue Oct 4, 2022 · 31 comments · Fixed by #1188
Closed
1 task done

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

akshatnema opened this issue Oct 4, 2022 · 31 comments · Fixed by #1188
Assignees
Labels
🐞 docs bug good first issue Good for newcomers Hacktoberfest Label issues as available for participants of https://hacktoberfest.digitalocean.com

Comments

@akshatnema
Copy link
Member

akshatnema commented Oct 4, 2022

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

  • I agree to follow this project's Code of Conduct
@akshatnema akshatnema added 🐞 docs bug Hacktoberfest Label issues as available for participants of https://hacktoberfest.digitalocean.com good first issue Good for newcomers labels Oct 4, 2022
@toukirkhan
Copy link
Contributor

@manavdesai27
Copy link
Contributor

Hey can I contribute to this issue? @akshatnema

@akshatnema
Copy link
Member Author

akshatnema commented Oct 4, 2022

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
Copy link
Contributor

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
Copy link
Member

quetzalliwrites commented Oct 4, 2022

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
Copy link
Member Author

akshatnema commented Oct 4, 2022

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
Copy link
Contributor

@manavdesai27 working on this one?

@manavdesai27
Copy link
Contributor

@manavdesai27 working on this one?

Yes, I'm working on this issue.

@toukirkhan
Copy link
Contributor

@manavdesai27 okay you take this one.

@manavdesai27
Copy link
Contributor

manavdesai27 commented Oct 5, 2022

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.0 as 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
Copy link
Member

magicmatatjahu commented Oct 5, 2022

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
Copy link
Member Author

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
Copy link
Member

derberg commented Oct 11, 2022

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
Copy link
Member Author

@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 😅.

@akshatnema akshatnema removed the Hacktoberfest Label issues as available for participants of https://hacktoberfest.digitalocean.com label Oct 11, 2022
@akshatnema akshatnema changed the title [Docs Bug 🐞 report]: Previous and UpNext Buttons are wrongly linked in some doc pages [Docs Bug 🐞 report]: Automate the Prev and UpNext buttons for Docs Oct 11, 2022
@derberg
Copy link
Member

derberg commented Oct 12, 2022

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.

@akshatnema akshatnema added the Hacktoberfest Label issues as available for participants of https://hacktoberfest.digitalocean.com label Oct 12, 2022
@manavdesai27
Copy link
Contributor

manavdesai27 commented Oct 12, 2022

@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
Copy link
Member Author

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

@derberg
Copy link
Member

derberg commented Dec 1, 2022

hey folks, IMHO we need 2 mechanisms:

  • one proposed by @akshatnema based on weight. So automatically detect what is next, and what previous
  • 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

wdyt?

@toukirkhan
Copy link
Contributor

Both options LGTM

@quetzalliwrites
Copy link
Member

  • 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 ? 😬😂😂

Copy link
Member

derberg commented Dec 12, 2022

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 😅

@quetzalliwrites quetzalliwrites moved this to Changes proposed 📄 ✨✨ in AsyncAPI Docs Board Dec 16, 2022
@akshatnema
Copy link
Member Author

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?

Copy link
Member

derberg commented Dec 21, 2022

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
Copy link
Member Author

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.

Copy link
Member

derberg commented Dec 21, 2022

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
Copy link
Member Author

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
Copy link
Member Author

image

Although 2 different log messages of same object 😅

Copy link
Member

derberg commented Dec 21, 2022

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

@akshatnema
Copy link
Member Author

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.

Copy link
Member

derberg commented Dec 21, 2022

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

@quetzalliwrites
Copy link
Member

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

@quetzalliwrites quetzalliwrites moved this from Changes proposed 📄 ✨✨ to Done 🚀 in AsyncAPI Docs Board Jan 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 docs bug good first issue Good for newcomers Hacktoberfest Label issues as available for participants of https://hacktoberfest.digitalocean.com
Projects
Status: Done 🚀
Development

Successfully merging a pull request may close this issue.

6 participants