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

Added Case Studies page in Overview Section #473

Merged
merged 13 commits into from
Mar 20, 2024

Conversation

TamannaVerma99
Copy link
Contributor

What kind of change does this PR introduce?
Added a new page in Overview section to show the case studies published in the blog. Currently, since I don't have access to the data file that has information related to blogs published, I have created a sample file of my own named casestudies.json with the same data format as mentioned in the issue and linked it to the card component. This makes the JSON file easily replaceable with the type of data we'll be using.

Issue Number:

Does this PR introduce a breaking change?
No

Copy link
Collaborator

@benjagm benjagm left a comment

Choose a reason for hiding this comment

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

This a really great first draft TamannaVerma99 !!!

I am leaving some comments to reorganize the solution but great job so far. Remember we'll use the card component that is being in process in #433

@@ -284,6 +286,7 @@ export const DocsNav = () => {
label='Similar Technologies'
/>
<DocLink uri='/overview/code-of-conduct' label='Code of Conduct' />
<DocLink uri='/overview/casestudies' label='Casestudies' />
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<DocLink uri='/overview/casestudies' label='Casestudies' />
<DocLink uri='/overview/casestudies' label='Case Studies' />

@@ -0,0 +1,7 @@
---
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of md file i think it will be better to create a tsx file this way: pages/overview/casestudies/index.page.tsx.

See an example of this type of page in the sponsors page inside overview. You can have the title and intro paragraph in a md file and later put the grid code in the tsx page. See implementations page for inspiration.

@@ -0,0 +1,24 @@
import React from 'react';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of a component for the whole list, we will be using a standard component created in this PR:#433

I think you are not going to need to create a component, instead use the one created by Michael-Obele in #433.

The logic to build the grid is something you will add in pages/overview/casestudies/index.page.tsx.

@TamannaVerma99
Copy link
Contributor Author

Thanks @benjagm !
I just want to reconfirm are we going to use standardized cards for this page too?As in the issue you mentioned we are not going to use standardized cards we are instead going to use them for welcome page.
I am working on other feedbacks you have given.Thanks :)

@benjagm
Copy link
Collaborator

benjagm commented Mar 6, 2024

Thanks @benjagm ! I just want to reconfirm are we going to use standardized cards for this page too?As in the issue you mentioned we are not going to use standardized cards we are instead going to use them for welcome page. I am working on other feedbacks you have given.Thanks :)

Thanks! In the initial requirements of this issue we did not speak about common card component, however I did it in this other comment of that issue. I realized that 3 people were working in a very similar problem so is better to standarize and reuse.

#433 (comment)

@benjagm benjagm mentioned this pull request Mar 6, 2024
33 tasks
@benjagm benjagm added this to the Docs Release 3 milestone Mar 6, 2024
@TamannaVerma99 TamannaVerma99 changed the base branch from main to web-release-3 March 7, 2024 14:19
@TamannaVerma99
Copy link
Contributor Author

Made the required changes @benjagm .
However, since I am unable to view the new web3-release-branch in my forked repo, pulling changes from that branch is a difficult affair. Also, even though the card component already exists merged in the target branch, it still shows the same as a new addition in my PR which am unable to understand. Rest, the working is completely fine.

Copy link
Collaborator

@benjagm benjagm left a comment

Choose a reason for hiding this comment

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

Amazing progress. We are really close.

Left some comments. There is extra space after the blue footer that we need to avoid. We fixed it for other pages some days ago.

Have you done a rebase to bring the changes from the new parent branch?

@@ -0,0 +1,59 @@
[
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you please build a real data set with our case studies? This is the list:
https://json-schema.org/blog?type=Case%20Study

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @benjagm
where do we want to direct the link we are adding in each casestudy card? According to card's implementation the link I am adding is shown as read more.
e.g if I have added homepage url in link prop of card it is showing read more and on clicking read more i am directed to home page.

Copy link
Collaborator

Choose a reason for hiding this comment

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

where do we want to direct the link we are adding in each casestudy card?

The url will be the case study blog post.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay! Got it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @benjagm I have some doubts regarding data set
What are we going to add at the place of logo and are we going to add complete title of blog post if yes then it will be too large because in the implementation of card title it is made by using h3 so how should I proceed with it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We need to find the company logos and add them to the folder: https://github.com/json-schema-org/website/tree/main/public/img/logos

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay! and what about the size of card title do I need to change the card component?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why changing the size of the card? Try to use this one and if you need to make some adjustments do it but just for the card features that you need. Remember that the card is now going to be used in 3 different pages.

For example you can change the logo size or fix a bug but not the whole card component without reason. Right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for the confusion @benjagm
I am not asking to change the card size but the bug is related to title if I directly put title in this component it will look like this

cr

It is because title is written with h3 size in card component.
I will definitely not change the card component directly but if there is no other way I will just make a copy of this component and make changes to that one .

Copy link
Collaborator

Choose a reason for hiding this comment

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

In this case feel free to modify it

components/Sidebar.tsx Outdated Show resolved Hide resolved
@@ -164,7 +164,12 @@ const MainNavigation = () => {
isActive={section === 'community'}
/>
<div className='flex max-sm:ml-4 items-center gap-6 md:gap-4'>
<div className='flex justify-center rounded border-2 border-gray-100 ml-0 w-[120px] md:w-full'>
<div
Copy link
Collaborator

Choose a reason for hiding this comment

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

You don't need to edit this page.

@@ -0,0 +1,53 @@
import React from 'react';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please use the Card component available in the branch and modify it if necessary.

@@ -0,0 +1,4 @@
../../../_includes/community/programs/casestudies/casestudies.md
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove this md file. Let's put all the content in the tsx page.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Working on it :)

import { SectionContext } from '~/context';
import data from 'data/casestudies.json';
import Card from '~/components/Card';
export async function getStaticProps() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's not use a md file here. Let's add all the content here.

<title>{newTitle}</title>
</Head>
<Headline1>{newTitle}</Headline1>
<StyledMarkdown markdown={blocks[0]} />
Copy link
Collaborator

Choose a reason for hiding this comment

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

It will be great to have a title and an intro paragraph. You can use the typical lorem ipsum.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback :)
I will soon implement all the points mentioned above.

@TamannaVerma99
Copy link
Contributor Author

Hey @benjagm .
Because of the issues caused in this PR since the branch was based on main instead of web-release-3.0, I have now created a new PR #540 with all the recommended changes. I would suggest closing this PR hereafter, and continuing our discussions on the other one. Thanks!

@TamannaVerma99 TamannaVerma99 requested a review from a team as a code owner March 20, 2024 09:36
Copy link
Collaborator

@benjagm benjagm left a comment

Choose a reason for hiding this comment

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

Amazing job!!! This is ready to go to the dev branch. I pushed some code to make some adjustments with the logos but really great work.

Congrats!!!!

@benjagm benjagm merged commit ee6bfdf into json-schema-org:web-release-3 Mar 20, 2024
2 checks passed
@TamannaVerma99
Copy link
Contributor Author

Amazing job!!! This is ready to go to the dev branch. I pushed some code to make some adjustments with the logos but really great work.

Congrats!!!!

Thanks:)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants