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 List component to the sistent components page #6086

Open
wants to merge 26 commits into
base: master
Choose a base branch
from

Conversation

csengineer1990
Copy link
Contributor

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

  • Yes, I signed my commits.

@github-actions github-actions bot added area/projects An issue relating to Layer5 initiatives (projects) project/sistent labels Nov 20, 2024
@l5io
Copy link
Contributor

l5io commented Nov 20, 2024

🚀 Preview for commit 9f5a6b7 at: https://673daeac4ab01b5bf2a3bc3c--layer5.netlify.app

@mdkaifansari04
Copy link
Contributor

image

You can consider adding all Types of List component into that navigation-menu

@csengineer1990
Copy link
Contributor Author

image

You can consider adding all Types of List components into that navigation-menu

@mdkaifansari04
Thank you for the feedback!

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?
Additionally, I noticed that the attached image isn’t visible on my end. If possible, could you reattach it or provide additional details? That would help me make the necessary updates more effectively.

Thank you!

@mdkaifansari04
Copy link
Contributor

image
If you look up to this side navigation menu, it has one menu as List Component. Try adding more menu items that is been presented in the overview section

@sudhanshutech
Copy link
Member

change the structure of page as per new changes made around adding sistent components

@csengineer1990
Copy link
Contributor Author

image If you look up to this side navigation menu, it has one menu as List Component. Try adding more menu items that is been presented in the overview section

Please check I have made the changes.

@l5io
Copy link
Contributor

l5io commented Nov 22, 2024

🚀 Preview for commit 3ab022b at: https://674009ea0d52eefebdee81a6--layer5.netlify.app

@csengineer1990
Copy link
Contributor Author

change the structure of page as per new changes made around adding sistent components

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!

@sudhanshutech
Copy link
Member

change the structure of page as per new changes made around adding sistent components

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.

@vishalvivekm
Copy link
Contributor

@csengineer1990
Thank you for your contribution!
Let's discuss this during the website call on Monday at 6:30 PM IST

adding it as an agenda item to the meeting minutes.

@Vidit-Kushwaha
Copy link
Contributor

@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";
Copy link
Contributor

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/

Copy link
Contributor

Choose a reason for hiding this comment

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

Same

Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above stated

Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Contributor

@Vidit-Kushwaha Vidit-Kushwaha left a 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.

@csengineer1990
Copy link
Contributor Author

Hi @csengineer1990 Facing issues merging your PR in Sistent?

We are conducting a special tutorial on this Monday, December 16, 2024, at 7 AM Central (6:30 PM IST) in the website meeting.

We'll cover:

* Navigating the Sistent repo

* Setting up locally

* Helping your initial pull request to be merged

Get more information: here

👥 Get involved:

* Explore the [Newcomers’ Form.](https://layer5.io/newcomer/)

* Check out the [meeting minutes.](https://bit.ly/2XK4eQV)

* Join the [community calendar](https://layer5.io/community/calendar#meetings).

Don’t miss out. 🚀

ok sir I will join. thanks

@l5io l5io temporarily deployed to commit December 15, 2024 12:04 Inactive
@csengineer1990
Copy link
Contributor Author

csengineer1990 commented Dec 16, 2024

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.

@l5io
Copy link
Contributor

l5io commented Dec 19, 2024

🚀 Preview for commit cc194c9 at: https://6764040ae6e81c05629a77b2--layer5.netlify.app

@l5io
Copy link
Contributor

l5io commented Dec 19, 2024

🚀 Preview for commit cc194c9 at: https://676404061e55280ef5313a0a--layer5.netlify.app

@csengineer1990
Copy link
Contributor Author

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.

@sudhanshutech
Copy link
Member

@csengineer1990
Let's discuss this during the website call today at 6:30 PM IST (7:00 AM CT).

Please add it as an agenda item to the meeting minutes.

Copy link
Contributor

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

Copy link
Contributor

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>
Copy link
Contributor

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

Suggested change
` <SistentThemeProvider>
` <SistentThemeProvider>

Copy link
Contributor

Choose a reason for hiding this comment

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

their is a a and unwanted indentation under the code section of the list It appears to be unpleasant.
image

/>
</div>
<div className="main-content">
<a id="List Components">
Copy link
Contributor

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

Suggested change
<a id="List Components">
<a id="List Components">

<li>List Subheader: Provides a labeled header for grouping related list items.</li>

</ul>
<a id="Types of List component">
Copy link
Contributor

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

Suggested change
<a id="Types of List component">
<a id="Types of List component">
Suggested change
<a id="Types of List component">
<a id="Types of List component">
<h3> Types of List component </h3>
</a>

Copy link
Contributor

@Vidit-Kushwaha Vidit-Kushwaha left a 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/projects An issue relating to Layer5 initiatives (projects) project/sistent
Development

Successfully merging this pull request may close these issues.

[Sistent] Add List component to the sistent components page
6 participants