fix: automated DocsButton #1188

akshatnema · 2022-12-23T11:59:22Z

Description
This PR consists of automating the DocsButton component for all the Doc pages. The Up Next and Go Back Buttons are automated according to the order of the pages given in the Doc Tree.

Related issue(s)
Fixes #1000

Signed-off-by: akshatnema <20bcs022@iiitdmj.ac.in>

netlify · 2022-12-23T11:59:27Z

✅ Deploy Preview for asyncapi-website ready!

Name	Link
🔨 Latest commit	`fd00679`
🔍 Latest deploy log	https://app.netlify.com/sites/asyncapi-website/deploys/63ca646e7a531a0008499d41
😎 Deploy Preview	https://deploy-preview-1188--asyncapi-website.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

github-actions · 2022-12-23T12:02:04Z

⚡️ Lighthouse report for the changes in this PR:

Category	Score
🔴 Performance	34
🟠 Accessibility	88
🟢 Best practices	100
🟢 SEO	100
🔴 PWA	30

Lighthouse ran on https://deploy-preview-1188--asyncapi-website.netlify.app/

Signed-off-by: akshatnema <20bcs022@iiitdmj.ac.in>

akshatnema · 2022-12-23T13:37:47Z

@alequetzalli Functionality implemented!! It is not as effective as manual insertion right now, but it will be improved as we need this improvement quite urgently. I have removed the manual insertion of components in the pages so far. I'll like to have your review first as Lukasz is on vacation right now.

akshatnema · 2023-01-10T12:52:30Z

@magicmatatjahu @derberg @alequetzalli Kindly look into the implementation and preview of this PR and review it. Till now, I found this to be a quick and effective solution to automate these buttons according to DocTree. Kindly suggest or give your feedback on the implementation made.

derberg

as we need this only for docs layout, why you added this new code to Layout.js?

components/layout/Layout.js

derberg · 2023-01-10T17:14:01Z

components/layout/Layout.js

+  const allDocPosts = posts.filter(p => p.slug.startsWith('/docs/'))
+  const treePosts = buildNavTree(allDocPosts)
+  let structuredPosts = []
+  const convertDocPosts = (docObject) => {


for future us 😄 , please add a comment what the purpose of this function is, why the conversion is needed and what is converted. I don't think you are converting anything, you are rather generating a flat list of all docs 🤔

akshatnema · 2023-01-10T20:07:46Z

as we need this only for docs layout, why you added this new code to Layout.js?

This is because DocsLayout is called after the doc post route has been requested. It contains a prop for each doc post that has been requested by the user. This means that for every render of a doc, the DocsLayout.js will work accordingly, whereas, in Layout.js Component, the logic has been used for all the doc posts at once. So, it is easier to work on the list of Doc Posts for once and add some fields accordingly.

Co-authored-by: Lukasz Gornicki <lpgornicki@gmail.com>

derberg

lgtm, please just add as many comments as possible 🙏🏼

things like index-2>=0 or index+1<countDocPages scare the hell out of me when I think that sometime I will have to go there and make some adjustments 😆 and will end up needing to debug every single line 😄

Signed-off-by: akshatnema <20bcs022@iiitdmj.ac.in>

akshatnema · 2023-01-12T10:05:42Z

@derberg Comments are added to the changes. You can now review the implementation and logic behind the buttons.

derberg

@akshatnema thanks ❤️

one final question, sorry. Wouldn't it be better if this is all calculated in https://github.com/asyncapi/website/blob/master/scripts/build-post-list.js

akshatnema · 2023-01-16T17:37:12Z

one final question, sorry. Wouldn't it be better if this is all calculated in master/scripts/build-post-list.js

Yeah, I have a plan of doing that in building the posts list, but right now I couldn't find the logic. Since, we need something right now, I build this solution in frontend 😅.

derberg · 2023-01-16T17:41:10Z

So you mean that once we merge, you will work on followup PR?

I'm ok to accept it. @magicmatatjahu ?

akshatnema · 2023-01-16T17:44:46Z

So you mean that once we merge, you will work on followup PR?

Yeah, because I need to do some hit and trials while creating and implementing the logic. I'll probably have to change the processing of posts list little bit to add these fields during the build time.

quetzalliwrites · 2023-01-17T01:46:39Z

@alequetzalli Functionality implemented!! It is not as effective as manual insertion right now, but it will be improved as we need this improvement quite urgently. I have removed the manual insertion of components in the pages so far. I'll like to have your review first as Lukasz is on vacation right now.

Heyo, @akshatnema ! this is so exciting! ✨✨✨✨✨

I was on vacation and just got back. ✌🏽

Signed-off-by: akshatnema <20bcs022@iiitdmj.ac.in>

…mate

akshatnema · 2023-01-17T06:07:11Z

@alequetzalli Done with removing buttons in the Welcome page.

quetzalliwrites

NOW FOR REAL 😄✨✨✨✨✨✨

akshatnema · 2023-01-18T13:01:53Z

@derberg your review is needed for this PR.

derberg · 2023-01-18T13:09:23Z

just please create followup issue for the implementation improvement

magicmatatjahu

You try to write such a logic inside Layout component and it should be written on building phase (in one of scripts) - atm every page on building phase and runtime is affected and we process unnecessary operations. In other words, please move logic to the scripts and add new metadata to the pages nodes.

akshatnema · 2023-01-18T13:18:38Z

In other words, please move logic to the scripts and add new metadata to the pages nodes.

Ok. That is my plan for the next follow up PR and I completely understand that current implementation will increase the run time of the website while rendering different pages. If you want the logic to be written in scripts folder only, you probably have to wait for some time as it requires some changes in the building of posts and the addition of some metadata nodes in docs. Also, as specified by @alequetzalli in this PR, we do need to figure out a way to add rootSection and Section names inside buttons so that a reader can identify properly which page the button is pointing to. But if you need this automation to be added quite urgently, I think you can go with this right now because it isn't increasing the runtime of the website too much. What's your opinion? @magicmatatjahu

magicmatatjahu · 2023-01-18T13:24:48Z

@akshatnema If we were to have a release by Friday then I would accept it and have no problem, but due to the fact that the website code (overall) looks terrible and is becoming less and less readable (plus we don't have releases and we have a lot of time) then I would prefer it to be done in this PR.

If other codeowners are in favour of such a code push, I can accept it, but let them rethink my thoughts above.

derberg · 2023-01-18T15:05:40Z

@magicmatatjahu I shared my feedback here already #1188 (comment). @akshatnema is aware of the issue that code is in a wrong place, but this is a quick fix to address missing buttons in generator documentation that was added recently.

this is a maintainer contribution, not a random contributor that just came for one contribution and throw code on our backs, but maintainer that will create a followup issue to improve the code.

So yeah, I trust we will soon get a follow up cleanup

akshatnema · 2023-01-19T11:36:04Z

@derberg your review needed!!

quetzalliwrites · 2023-01-20T03:52:25Z

@derberg your review needed!!

YEAH LUKASZ, GET OVER HERE STAT!!

derberg

go go go
no need to wait for me if @alequetzalli and @magicmatatjahu already gave a green light

akshatnema · 2023-01-20T12:59:38Z

/rtm

automated DocsButton

be86f7f

Signed-off-by: akshatnema <20bcs022@iiitdmj.ac.in>

akshatnema requested review from fmvilas, derberg, mcturco, magicmatatjahu and asyncapi-bot-eve as code owners December 23, 2022 11:59

akshatnema and others added 2 commits December 23, 2022 17:32

Merge branch 'master' into automate

ebb5543

DocsButton removed from MD files

efea397

Signed-off-by: akshatnema <20bcs022@iiitdmj.ac.in>

akshatnema requested a review from quetzalliwrites as a code owner December 23, 2022 12:32

Merge branch 'master' into automate

fb5cc9c

derberg reviewed Jan 10, 2023

View reviewed changes

Update components/layout/Layout.js

e6e0707

Co-authored-by: Lukasz Gornicki <lpgornicki@gmail.com>

derberg reviewed Jan 11, 2023

View reviewed changes

akshatnema and others added 2 commits January 12, 2023 13:58

comments added

791e0c1

Signed-off-by: akshatnema <20bcs022@iiitdmj.ac.in>

Merge branch 'master' into automate

6143e44

Merge branch 'master' into automate

dbcd3ba

derberg reviewed Jan 16, 2023

View reviewed changes

Merge branch 'master' into automate

5af6aec

Merge branch 'master' into automate

73baff4

quetzalliwrites assigned akshatnema Jan 17, 2023

quetzalliwrites and others added 3 commits January 16, 2023 20:43

Merge branch 'master' into automate

b7cad08

removed button from Welcome page

3fea7e9

Signed-off-by: akshatnema <20bcs022@iiitdmj.ac.in>

Merge remote-tracking branch 'refs/remotes/origin/automate' into auto…

a3ebff5

…mate

akshatnema added 2 commits January 17, 2023 16:22

Merge branch 'master' into automate

52aa255

Merge branch 'master' into automate

78c7779

quetzalliwrites approved these changes Jan 18, 2023

View reviewed changes

akshatnema added 2 commits January 18, 2023 18:09

Merge branch 'master' into automate

398d02b

Merge branch 'master' into automate

8957674

Merge branch 'master' into automate

a90217b

magicmatatjahu requested changes Jan 18, 2023

View reviewed changes

magicmatatjahu approved these changes Jan 18, 2023

View reviewed changes

akshatnema added 2 commits January 18, 2023 22:07

Merge branch 'master' into automate

35da40f

Merge branch 'master' into automate

233ed9e

derberg approved these changes Jan 20, 2023

View reviewed changes

Merge branch 'master' into automate

fd00679

asyncapi-bot added the ready-to-merge label Jan 20, 2023

asyncapi-bot merged commit 1b82874 into asyncapi:master Jan 20, 2023

Shofiya2003 mentioned this pull request Jan 28, 2023

Extend workflow responsible for spec update in website repo with button addition asyncapi/spec#789

Closed

akshatnema mentioned this pull request Jan 30, 2023

fix: addition of Prev/Next buttons in build time #1273

Merged

akshatnema deleted the automate branch October 30, 2023 09:46

sambhavgupta0705 mentioned this pull request Apr 21, 2024

[Docs Bug 🐞 report]: prev/next buttons missing on new generator tool docs #1176

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: automated DocsButton #1188

fix: automated DocsButton #1188

akshatnema commented Dec 23, 2022

netlify bot commented Dec 23, 2022 •

edited

Loading

github-actions bot commented Dec 23, 2022 •

edited

Loading

akshatnema commented Dec 23, 2022

akshatnema commented Jan 10, 2023

derberg left a comment

derberg Jan 10, 2023

akshatnema Jan 10, 2023

akshatnema commented Jan 10, 2023

derberg left a comment

akshatnema commented Jan 12, 2023

derberg left a comment

akshatnema commented Jan 16, 2023

derberg commented Jan 16, 2023

akshatnema commented Jan 16, 2023

quetzalliwrites commented Jan 17, 2023

akshatnema commented Jan 17, 2023

quetzalliwrites left a comment

akshatnema commented Jan 18, 2023

derberg commented Jan 18, 2023

magicmatatjahu left a comment •

edited

Loading

akshatnema commented Jan 18, 2023 •

edited

Loading

magicmatatjahu commented Jan 18, 2023

derberg commented Jan 18, 2023

akshatnema commented Jan 19, 2023

quetzalliwrites commented Jan 20, 2023

derberg left a comment

akshatnema commented Jan 20, 2023

fix: automated DocsButton #1188

fix: automated DocsButton #1188

Conversation

akshatnema commented Dec 23, 2022

netlify bot commented Dec 23, 2022 • edited Loading

✅ Deploy Preview for asyncapi-website ready!

github-actions bot commented Dec 23, 2022 • edited Loading

akshatnema commented Dec 23, 2022

akshatnema commented Jan 10, 2023

derberg left a comment

Choose a reason for hiding this comment

derberg Jan 10, 2023

Choose a reason for hiding this comment

akshatnema Jan 10, 2023

Choose a reason for hiding this comment

akshatnema commented Jan 10, 2023

derberg left a comment

Choose a reason for hiding this comment

akshatnema commented Jan 12, 2023

derberg left a comment

Choose a reason for hiding this comment

akshatnema commented Jan 16, 2023

derberg commented Jan 16, 2023

akshatnema commented Jan 16, 2023

quetzalliwrites commented Jan 17, 2023

akshatnema commented Jan 17, 2023

quetzalliwrites left a comment

Choose a reason for hiding this comment

akshatnema commented Jan 18, 2023

derberg commented Jan 18, 2023

magicmatatjahu left a comment • edited Loading

Choose a reason for hiding this comment

akshatnema commented Jan 18, 2023 • edited Loading

magicmatatjahu commented Jan 18, 2023

derberg commented Jan 18, 2023

akshatnema commented Jan 19, 2023

quetzalliwrites commented Jan 20, 2023

derberg left a comment

Choose a reason for hiding this comment

akshatnema commented Jan 20, 2023

netlify bot commented Dec 23, 2022 •

edited

Loading

github-actions bot commented Dec 23, 2022 •

edited

Loading

magicmatatjahu left a comment •

edited

Loading

akshatnema commented Jan 18, 2023 •

edited

Loading