feat: added Faq section in overview #429
Conversation
| @@ -0,0 +1,25 @@ | |||
| import React, { useState, useEffect } from 'react' | |||
There was a problem hiding this comment.
-
This page will go under the overview section and with a link in the sidebar.
pages/overview/faq -
Can we rename frequently-asked-questions by faq?
-
This page is missing the sidebar. Please see [slug].page.tsx inside overview to understand how to show it.
-
I'd love to provide a experience like to this page: https://graphql.org/faq/ With groups, collapsing expanding effect and each question will have a mardown heading to able to generate self-links so people can link the questions externaly.
There was a problem hiding this comment.
This is also done
|
Thanks @benjagm for the review will implement these changes |
|
Hey @benjagm can you review this |
|
Can you please change the target branch of this PR to : https://github.com/json-schema-org/website/tree/web-release-3 |
I've changed the base branch can you review this now |
There was a problem hiding this comment.
This is almost done! Excellent job here @Akshaybagai52 !!
Just left the last changes but this is pretty close to be completed.
components/Sidebar.tsx
Outdated
| @@ -69,6 +69,7 @@ const getDocsPath = [ | |||
| '/overview/sponsors', | |||
| '/overview/similar-technologies', | |||
| '/overview/code-of-conduct', | |||
| 'overview/faq', | |||
There was a problem hiding this comment.
| 'overview/faq', | |
| '/overview/faq', |
There was a problem hiding this comment.
I've updated this
components/Faq.tsx
Outdated
| @@ -0,0 +1,18 @@ | |||
| import React from 'react'; | |||
There was a problem hiding this comment.
If accordion and faq are not reusable components I suggest to keep the functionality inside the faq tsx page.
There was a problem hiding this comment.
I made it like that because in future it can become reusable components. If any other page needs this accordion and also we can add the question answer by passing category as props so it can be reused if we have some other categories of questions like we are doing with "general"
components/Accordion.tsx
Outdated
| className='flex justify-between items-center p-4 cursor-pointer' | ||
| onClick={() => handleToggle(index)} | ||
| > | ||
| <div className='text-[20px]'>{item.question}</div> |
There was a problem hiding this comment.
For questions to have their own externally accesible url well need to add a self link something like:
<div className='text-[20px]'><a href="#{question-id}">{item.question}</a></div>
There was a problem hiding this comment.
What I did in this is that on clicking of question the accordion expands and then show the answer of the quesiton but with this addition it will redirect to other link.
So what I did in this is to add arrow in the end of answer on clicking of that arrow they can redirect to respected links.
Let me know about your views on this
There was a problem hiding this comment.
The accordion expansion effects works well but we need to also change the url from /faq to something like /faq#xxxxxxxxxx. The blue arrow you added I think doesn't add any value. Just focus on adding the self link navigation when expanding the accordion.
There was a problem hiding this comment.
Got it I'll update this
| @@ -0,0 +1,38 @@ | |||
| name: Greet on User First PR Merge | |||
There was a problem hiding this comment.
Why pushing this change? please remove from the PR anything that is unrelated to this fature.
There was a problem hiding this comment.
actually these changes comes automatically when I change the base branch
There was a problem hiding this comment.
That is fine but remove them from your PR. Just push your code
components/CarbonsAds.tsx
Outdated
| @@ -115,6 +115,12 @@ CarbonAds.stylesheet = { | |||
| margin-top: 4px; | |||
| color: rgb(100 116 139); | |||
| } | |||
|
|
|||
| @media (max-width: 1023px) { | |||
There was a problem hiding this comment.
Why pushing this change? please remove from the PR anything that is unrelated to this feature.
components/Layout.tsx
Outdated
| @@ -329,128 +329,6 @@ const Footer = () => ( | |||
| </footer> | |||
| ); | |||
|
|
|||
| const OpenJS = () => ( | |||
There was a problem hiding this comment.
Why pushing this change? please remove from the PR anything that is unrelated to this feature.
| @@ -99,30 +99,31 @@ Using the product catalog example, `productId` is a numeric value that uniquely | |||
| To add the `properties` object to the schema: | |||
|
|
|||
| 1. Add the `properties` validation keyword to the end of the schema: | |||
|
|
|||
| ```json | |||
|
|
|||
There was a problem hiding this comment.
Why pushing this change? please remove from the PR anything that is unrelated to this feature.
| @@ -139,8 +139,8 @@ you control, for example: | |||
| { "$id": "http://yourdomain.com/schemas/myschema.json" } | |||
There was a problem hiding this comment.
Why pushing this change? please remove from the PR anything that is unrelated to this feature.
| @@ -8,7 +8,7 @@ section: docs | |||
| * [Schema Identification](#schema-identification) | |||
There was a problem hiding this comment.
Why pushing this change? please remove from the PR anything that is unrelated to this feature.
components/Accordion.tsx
Outdated
| className='flex justify-between items-center p-4 cursor-pointer' | ||
| onClick={() => handleToggle(index)} | ||
| > | ||
| <div className='text-[20px]'>{item.question}</div> |
There was a problem hiding this comment.
The accordion expansion effects works well but we need to also change the url from /faq to something like /faq#xxxxxxxxxx. The blue arrow you added I think doesn't add any value. Just focus on adding the self link navigation when expanding the accordion.
|
Hey @benjagm these changes comes automatically when I changed the base branch from main to web-release-3. Can you tell me do I need to take pull from web-release-3 or I would take from main ? |
Akshaybagai52
changed the title
Thanks for the detailed review I will fix this |
Akshaybagai52
force-pushed
the
feature/faq-section
branch
from
b0c19e4 to
da67586
Compare
What kind of change does this PR introduce?
Issue Number: #190
Screenshots/videos:
If relevant, did you update the documentation?
Summary
Does this PR introduce a breaking change?