-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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 List component to the sistent components page #6086
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Roshan Goswami <[email protected]>
Signed-off-by: Roshan <[email protected]>
🚀 Preview for commit 9f5a6b7 at: https://673daeac4ab01b5bf2a3bc3c--layer5.netlify.app |
@mdkaifansari04 I’m not entirely sure where you’d like me to make the suggested changes regarding adding all types of List components to the navigation menu. Could you please provide more clarification? Thank you! |
change the structure of page as per new changes made around adding sistent components |
Signed-off-by: Roshan Goswami <[email protected]>
🚀 Preview for commit 3ab022b at: https://674009ea0d52eefebdee81a6--layer5.netlify.app |
Could you please elaborate on what you mean by changing the structure of the page with the new Sistent components? I’d appreciate it if you could guide me on where exactly the changes need to be made or what specific updates are required. This will help me ensure that I address your requirements accurately. Thank you! |
you don't have to create dir under pages , it will automatically created. |
@csengineer1990 adding it as an agenda item to the meeting minutes. |
Signed-off-by: l5io <[email protected]>
Signed-off-by: l5io <[email protected]>
Signed-off-by: l5io <[email protected]>
Signed-off-by: Roshan Goswami <[email protected]>
Signed-off-by: l5io <[email protected]>
Signed-off-by: l5io <[email protected]>
@csengineer1990 We will be having a sistent tutorial on this coming Monday (16 Dec 2024) ; this might help you to merge your PR |
@@ -0,0 +1,8 @@ | |||
import React from "react"; |
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.
This file is not required, as pages will be dynamically created. so you don't require to create any pages under this dirc src/pages/projects/sistent/components/list/
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.
Same
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.
Same as above stated
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.
Rather than putting list data on componentsData, their is another file called content.js under same dirc src/sections/Projects/Sistent/components/
you have to put necessay data.
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.
ok sir I will check and make the required changes.
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.
I have included a few reviews.
Signed-off-by: l5io <[email protected]>
Signed-off-by: l5io <[email protected]>
Signed-off-by: Roshan Goswami <[email protected]>
I am currently working on addressing the requested changes based on the review. This includes resolving some conflicts and updating the repository. The pull request is still open, and I am actively working on completing it. Thank you for your patience. |
Signed-off-by: l5io <[email protected]>
Signed-off-by: Roshan Goswami <[email protected]>
Signed-off-by: l5io <[email protected]>
…nges Signed-off-by: Roshan Goswami <[email protected]>
🚀 Preview for commit cc194c9 at: https://6764040ae6e81c05629a77b2--layer5.netlify.app |
🚀 Preview for commit cc194c9 at: https://676404061e55280ef5313a0a--layer5.netlify.app |
Could you kindly review it again? I have made the changes as per your previous feedback. If I missed anything or made any mistakes, please let me know and guide me on how I can correct them. |
@csengineer1990 Please add it as an agenda item to the meeting minutes. |
feature_data.json
Outdated
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.
revert this file, i.e. feature_data.json
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.
Same
|
||
const codes = [ | ||
// Basic List with List Items | ||
` <SistentThemeProvider> |
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 you remove SistentThemeProvider from code example blocks
` <SistentThemeProvider> | |
` <SistentThemeProvider> |
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.
ok, sir.
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.
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.
ok, sir.
/> | ||
</div> | ||
<div className="main-content"> | ||
<a id="List Components"> |
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.
Hey, can you remove the unnecessary anchor tag in the list page? One minor recommendation is that the title will always appear in the sidebar when it is placed inside the h2 tag. Make sure you use it sensibly. only put important heading
<a id="List Components"> | |
<a id="List Components"> |
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.
ok, sir.
<li>List Subheader: Provides a labeled header for grouping related list items.</li> | ||
|
||
</ul> | ||
<a id="Types of List component"> |
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.
How a link should be added to the sidebar
<a id="Types of List component"> | |
<a id="Types of List component"> |
<a id="Types of List component"> | |
<a id="Types of List component"> | |
<h3> Types of List component </h3> | |
</a> |
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.
ok, sir.
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.
Hey @csengineer1990 I have reviewed your PR and provided some feedback
Signed-off-by: Roshan Goswami <[email protected]>
🚀 Preview for commit 3722bc5 at: https://676bfac048e734388c266c4d--layer5.netlify.app |
I’ve made the changes as per your feedback, including updates to the index and code.js files. I’ve also reverted the feature_data.json file at both locations. Please review and let me know if anything needs further adjustment, or if I made any mistakes while reverting the file. |
🚀 Preview for commit eb021c2 at: https://676c057a6c5eeb40be80cd4f--layer5.netlify.app |
Signed-off-by: l5io <[email protected]>
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.
It Looks good to me
</List> `, | ||
// List with Action Buttons | ||
` <List> | ||
<ListItemButton onClick={() => alert("Clicked!")}> Layer5 Sistent Action 1</ListItemButton> |
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.
@csengineer1990 If you could provide a nested list code example, here is example link
Description
This PR fixes #5951
By adding the List component to the Sistent Components page.
The required content and documentation for the List component have been included, along with updates to the necessary files.
Notes for Reviewers
I have made the following changes:
Added the List of component implementation and documentation.
Updated the related files to reflect these changes.
Could you review the updates and let me know if any additional changes or content updates are required?
Signed commits