-
Notifications
You must be signed in to change notification settings - Fork 572
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 the hide blog feature #50
Added the hide blog feature #50
Conversation
yourData.json named "hideBlog". and hid the blog values from navigation.
✅ Deploy Preview for react-portfolio-template ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome
Looks Great!
Go ahead And merge Away 🚀 |
I can't merge this. I don't have write access, only you can merge |
Hi nice job! Just a question, if someone were to put in /blog after the URL, wouldn't the route still exist? We are only removing the button here. Maybe we can work on this enhancement later. A very minor issue tbh, great work on the commit 😄 |
@@ -120,6 +120,9 @@ export default { | |||
aboutpara: | |||
"Lorem Ipsum is simply dummy text of the printing and typesetting industry. Lorem Ipsum has been the industry's standard dummy text ever since the 1500s, when an unknown printer took a galley of type and scrambled it to make a type specimen book. It has survived not only five centuries, but also the leap into electronic typesetting, remaining essentially unchanged. It was popularised in the 1960s with the release of Letraset sheets containing Lorem Ipsum passages, and more recently with desktop publishing software like Aldus PageMaker including versions of Lorem Ipsum.", | |||
// End About Section --------------------- | |||
|
|||
// Hide Blog section | |||
hideBlog: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be moved to the top of the json file? It may be more helpful to keep it at the top.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this makes sense.
{data.name}. | ||
</h1> | ||
{!isBlog ? ( | ||
<div className="flex"> | ||
<Button onClick={handleWorkScroll}>Work</Button> | ||
<Button onClick={handleAboutScroll}>About</Button> | ||
<Button onClick={() => router.push("/blog")}>Blog</Button> | ||
{!data.hideBlog && ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we destructure this variable from the data
object? Just makes it a bit more readable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@achu-krishna Once you are done with this resolution we can merge!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chetanverma16 @Aryan3212 Sure I will do it ASAP
I noticed that too. We could create a new issue for that |
@achu-krishna For sure! |
Yeah, @achu-krishna Nice Work! |
Add a new field in the
yourData.json
namedhideBlog
which is a boolean field.In the
Header/index.js
used logical and operator to hide the navItem