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

feat: add table of contents in case study #1673

Merged
merged 32 commits into from
Dec 20, 2023

Conversation

Shurtu-gal
Copy link
Contributor

@Shurtu-gal Shurtu-gal commented May 15, 2023

Description

  • Refactored case study page's code to decrease code repetition.
  • Added dynamic table of contents.

Related issue(s)
Fixes #1502 #1505

@netlify
Copy link

netlify bot commented May 15, 2023

Deploy Preview for asyncapi-website ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 7d3565c
🔍 Latest deploy log https://app.netlify.com/sites/asyncapi-website/deploys/6582f31a4d992a00087c5f7d
😎 Deploy Preview https://deploy-preview-1673--asyncapi-website.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

@Shurtu-gal
Copy link
Contributor Author

@derberg I have made the required changes.

@github-actions
Copy link

github-actions bot commented May 15, 2023

⚡️ Lighthouse report for the changes in this PR:

Category Score
🔴 Performance 21
🟢 Accessibility 98
🟢 Best practices 100
🟢 SEO 100
🔴 PWA 30

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

components/layout/CaseStudyLayout.js Outdated Show resolved Hide resolved
components/layout/CaseStudyLayout.js Outdated Show resolved Hide resolved
@akshatnema
Copy link
Member

@Shurtu-gal When clicking on any of the TOC of the webpage, it doesn't get bold. Kindly look into it.

@Shurtu-gal
Copy link
Contributor Author

@Shurtu-gal When clicking on any of the TOC of the webpage, it doesn't get bold. Kindly look into it.

@akshatnema Done.

derberg
derberg previously approved these changes May 22, 2023
Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect ❤️

@derberg
Copy link
Member

derberg commented May 22, 2023

@akshatnema please approve

Copy link
Member

@akshatnema akshatnema left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot_2023-05-22-21-04-19-81.jpg

Case study webpage becomes highly unresponsive in mobile view. Kindly correct it.

@Shurtu-gal
Copy link
Contributor Author

Screenshot_2023-05-22-21-04-19-81.jpg

Case study webpage becomes highly unresponsive in mobile view. Kindly correct it.

There was no problem from my end. Can you tell the specific screen size for this?

Copy link
Member

@Shurtu-gal This is the mobile view of the webpage and this error is on every mobile screen view.

@asyncapi-bot
Copy link
Contributor

asyncapi-bot commented Nov 8, 2023

⚡️ Lighthouse report for the changes in this PR:

Category Score
🔴 Performance 27
🟢 Accessibility 98
🟢 Best practices 92
🟢 SEO 100
🔴 PWA 33

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

@Shurtu-gal
Copy link
Contributor Author

@Mayaleeeee I have made the required changes. I am sorry that this took so much time 😢.

@Mayaleeeee
Copy link
Member

Mayaleeeee commented Nov 9, 2023

@Mayaleeeee I have made the required changes. I am sorry that this took so much time 😢.

No worries, @Shurtu-gal.

So here are my reviews:

  1. The chevron-down icon you used is supposed to be on the right side and given enough spacing because it's too close to the text. Please check the navigation bar texts with that icon to see how it's positioned and done.

  2. That's not the right colour to use. Please use hex code #242929, which is Grey-500.
    Screenshot 2023-11-09 101110

@Shurtu-gal
Copy link
Contributor Author

@Mayaleeeee I have made the required changes. I am sorry that this took so much time 😢.

No worries, @Shurtu-gal.

So here are my reviews:

  1. The chevron-down icon you used is supposed to be on the right side and given enough spacing because it's too close to the text. Please check the navigation bar texts with that icon to see how it's positioned and done.
  2. That's not the right colour to use. Please use hex code #242929, which is Grey-500.

@Mayaleeeee The requested changes are done.

@Mayaleeeee
Copy link
Member

Mayaleeeee commented Nov 9, 2023

@Mayaleeeee I have made the required changes. I am sorry that this took so much time 😢.

No worries, @Shurtu-gal.
So here are my reviews:

  1. The chevron-down icon you used is supposed to be on the right side and given enough spacing because it's too close to the text. Please check the navigation bar texts with that icon to see how it's positioned and done.
  2. That's not the right colour to use. Please use hex code #242929, which is Grey-500.

@Mayaleeeee The requested changes are done.

Looks good. However, I noticed that the active color (black) does not remain visible on "More details about AsyncAPI" until I scroll down to "Schema". Please update this to ensure it remains visible. @Shurtu-gal

@Shurtu-gal
Copy link
Contributor Author

@Mayaleeeee Done now the parent remains active until another nav item at the same level doesn't pass through the viewport.

image

Copy link
Member

@akshatnema akshatnema left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @Shurtu-gal , I do have a feature issue here 😅. As we scroll down, the menus should get expanded and closed automatically. But don't worry for it in this PR. We can target this feature in another PR or issue. LGTM ✔️

@akshatnema
Copy link
Member

@Mayaleeeee your review required!!!!

@Shurtu-gal
Copy link
Contributor Author

As we scroll down, the menus should get expanded and closed automatically.

I will try to resolve this too and open a PR as soon as possible.

@derberg
Copy link
Member

derberg commented Dec 19, 2023

imho we can merge, Maya had enough time to do review. If there are some issues, we can always follow up later

@akshatnema
Copy link
Member

imho we can merge, Maya had enough time to do review. If there are some issues, we can always follow up later

@derberg, @Mayaleeeee has requested changes on this PR. She has to approve it then. Should I bypass the branch protections to merge the PR?

@Mayaleeeee
Copy link
Member

Mayaleeeee commented Dec 20, 2023

imho we can merge, Maya had enough time to do review. If there are some issues, we can always follow up later

@derberg, @Mayaleeeee has requested changes on this PR. She has to approve it then. Should I bypass the branch protections to merge the PR?

Yes, please.
I'm viewing on mobile and I can't really see anything unless I view on my laptop.

If I see any changes that needs to be made from the laptop view, I'll update this comment.

@akshatnema
Copy link
Member

@Mayaleeeee Can you please approve the PR, since you requested changes before?

@asyncapi-bot asyncapi-bot merged commit bb67e4c into asyncapi:master Dec 20, 2023
19 checks passed
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.

Add table of contents to case studies view
6 participants