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

Add Comment #16

Merged
merged 31 commits into from
Nov 27, 2023
Merged

Add Comment #16

merged 31 commits into from
Nov 27, 2023

Conversation

TajwarSaiyeed
Copy link
Contributor

@TajwarSaiyeed TajwarSaiyeed commented Nov 9, 2023

Added comment explanations to Accordion, Alert, Avatar and Badge Component in the React app. The comments aim to enhance code readability and make it easier for developers to understand the functionality of those Components. This improvement is intended to facilitate future maintenance and collaboration.

Changes:

  • [✅] Added comments
  • [✅] Clarified complex logic and functionality.
  • [✅] Improved overall code documentation.

Components

  • [✅] Accordion
  • [✅] Alert
  • [✅] Avatar
  • [✅] Badge

Utility Function

  • [✅] managing search data

Helper Function

  • [✅] exclude.ts
  • [✅] floating.ts
  • [✅] range.ts
  • [✅] rangeWithDots.ts
  • [✅] use-Floating.ts
  • [✅] uuid.ts
  • [✅] window-exists.ts

Copy link

netlify bot commented Nov 9, 2023

Deploy Preview for keep-react-sm ready!

Name Link
🔨 Latest commit ac150e1
🔍 Latest deploy log https://app.netlify.com/sites/keep-react-sm/deploys/6562a2a4d77a7500093e8fbb
😎 Deploy Preview https://deploy-preview-16--keep-react-sm.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@Arifulislam5577
Copy link
Collaborator

Thank you @TajwarSaiyeed for your contribution,
Your feedback on the accordion component is greatly appreciated. If you're interested, we'd love to have your insights on other components as well. Your contributions make a significant impact, and we value your involvement.

@TajwarSaiyeed
Copy link
Contributor Author

@Arifulislam5577
Thank you for the warm feedback! I'm glad my work on the accordion component is helpful. I'm definitely interested in contributing more and providing insights on other components. Excited to be a part of the project and looking forward to more collaborations!

@TajwarSaiyeed
Copy link
Contributor Author

@Arifulislam5577

  • [✅] module-not-found path fix

image

@TajwarSaiyeed
Copy link
Contributor Author

TajwarSaiyeed commented Nov 19, 2023

@Arifulislam5577 @kausarpial
Hi,
Dear kindly check the update. I have added some of the components of keep react. kindly check and update it to main and also one thing that is

i will update all the remaining components.

@@ -2,22 +2,48 @@ import type { Meta, StoryObj } from '@storybook/react'
import { Accordion } from '.'
import { removeFragment } from '../../helpers/mergeDeep'

/**
* Meta information for the Accordion component.
*/
const meta: Meta<typeof Accordion> = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please don't add comments in any *.stories.tsx file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okey, I will Modify All the *.stories.tsx file and remove the comment

Copy link
Collaborator

@Arifulislam5577 Arifulislam5577 left a comment

Choose a reason for hiding this comment

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

No need to add comments to the css interface. Avoid stories.tsx and theme.ts file. Please add comment only component props.

like this

alwaysOpen?: boolean
/**

  • Determines whether to show the open/close icon for each accordion panel.
  • @type {boolean}
    */

*
* @interface AccordionProps
* @extends {PropsWithChildren<ComponentProps<'div'>>}
*/
export interface AccordionProps extends PropsWithChildren<ComponentProps<'div'>> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

It was good. Please follow this style and add comment only component props not any other code.

/**
* Interface representing the theme object for the KeepAccordionTitle component.
*/

export interface keepAccordionTitleTheme {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't do it and avoid comment in this type of interface. There is no need to add comments to the css interface.

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 review. i will modify all the commented file and will focus to not add the commnet into the *.stoies.tsx and *theme.tsx file

only focus to explain the component file

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good, Carry on.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Kindly check the latest commit, i have removed unnecessaries commnet, add comment only to component

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi @TajwarSaiyeed,

Thank you for your valuable contribution! Your work will greatly benefit fellow developers navigating our component library. Making the codebase more readable directly from the component is a fantastic improvement.

However, I noticed two different comment styles, and it might be a bit confusing for contributors. If you could follow the first comment style you used in the accordion component, it would be excellent. Please follow this style and add only this 3 line as a comment, For example:

/**
 * Determines whether the accordion should always be open.
 * @type boolean
 * @default false
 */
alwaysOpen?: boolean;

Thanks again for your efforts!

Best regards,
Md Ariful Islam

Copy link
Contributor Author

@TajwarSaiyeed TajwarSaiyeed Nov 23, 2023

Choose a reason for hiding this comment

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

@Arifulislam5577
Thank you for the review, you will see the changes soon.

@TajwarSaiyeed TajwarSaiyeed changed the title Added: Comment/Explanation Accordion Component Add Comment Nov 23, 2023
@TajwarSaiyeed
Copy link
Contributor Author

@Arifulislam5577
Hi Dear,
Hope you are doing well. kindly check the latest update. All component modified as you suggestion. It can be possible there will mistake, kindy update me if any mistake and also any suggestion if you have.

If all ok then update it

@Arifulislam5577
Copy link
Collaborator

Hi @TajwarSaiyeed,

Thank you for your valuable contribution to our component library. Your efforts are greatly appreciated, and we look forward to seeing more contributions from you in the future. Keep up the excellent work!

Best regards,
Md Ariful Islam

@Arifulislam5577 Arifulislam5577 merged commit 01cd26f into StaticMania:main Nov 27, 2023
4 checks passed
@TajwarSaiyeed
Copy link
Contributor Author

Hi @TajwarSaiyeed,

Thank you for your valuable contribution to our component library. Your efforts are greatly appreciated, and we look forward to seeing more contributions from you in the future. Keep up the excellent work!

Best regards, Md Ariful Islam

Hi Md Ariful Islam,

Thank you so much for your encouraging words! I'm thrilled to be part of the "keep react" project, and I'm glad my contribution was valuable. It's truly motivating to receive such positive feedback.

I'm eager to continue contributing to the project and have some exciting ideas in mind. I'm planning to work on creating a new card design (glass card, sliding card, ribon card, etc) and a hero component etc. now. I believe these additions will enhance the overall user experience and visual appeal of the project.

If there are specific requirements or guidelines for these components, please let me know. I want to ensure that my contributions align seamlessly with the project's vision.

Looking forward to your guidance and to sharing more great work with the team.

Best regards,
Tajwar Saiyeed Abid

@Arifulislam5577
Copy link
Collaborator

Hi @TajwarSaiyeed,

Thank you for your enthusiastic message and, more importantly, for your valuable contributions to the "keep react" project. Your dedication and creativity are truly commendable!

We appreciate your eagerness to contribute further, and your proposed ideas for new card designs and hero components sound intriguing. However, currently, we are working within the constraints of our keep Design System, and adding new components that are not part of our Figma design may create inconsistencies.

We have an open issue on GitHub related to this, and your expertise could be invaluable in addressing and resolving this challenge. Alternatively, you're more than welcome to propose enhancements or features for our existing components, which align with our current design system.

We value your commitment and innovative spirit, and we're looking forward to seeing more of your impactful contributions. If you have any questions or need guidance, feel free to reach out.

Best regards,
Md Ariful Islam

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.

2 participants