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

[Components] Add sistent naming convention for our components #465

Closed

Conversation

sudhanshutech
Copy link
Member

Notes for Reviewers

This PR fixes #

For being consistent we can use the sistent naming convention instead of mui for exporting these theme based components.

Signed commits

  • Yes, I signed my commits.

@sudhanshutech
Copy link
Member Author

@nebula-aac we discussed in meet that it is good to use our own naming convention while exporting these components . Make sense to you @nebula-aac ?

Copy link
Contributor

@nebula-aac nebula-aac left a comment

Choose a reason for hiding this comment

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

So we're only just naming the custom components, not the exported components to be used in Meshery?

@sudhanshutech
Copy link
Member Author

So we're only just naming the custom components, not the exported components to be used in Meshery?

They might because i think these styled component will only play major role when wrapped with sistent provider right!.
The naming is for only to keep our sistent codebase looks like its our own components and colors.

@@ -1,7 +1,7 @@
import { Components, Theme } from '@mui/material';
import { CHARCOAL, connected, darkSlateGray } from '../colors';

export const MuiCheckbox: Components<Theme>['MuiCheckbox'] = {
export const SistentCheckbox: Components<Theme>['MuiCheckbox'] = {
Copy link
Contributor

Choose a reason for hiding this comment

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

@sudhanshutech using sistent as prefix is redundant ( as they will be scoped under sistent ) , I would suggest use Base-<Component> for unstyled / customizable component and <Functionality-<Component> for customized and ready to use ones , @nebula-aac whats your opinion ?

Copy link
Contributor

@nebula-aac nebula-aac Feb 3, 2024

Choose a reason for hiding this comment

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

It can get a bit long-winded in trying to import the components by names if they were set like that. If this is for internal use, perhaps maybe, but when it comes to exporting, we don't want the naming convention to be too complex to where errors happen because of spellings or one component was meant for something else.

Copy link
Contributor

Choose a reason for hiding this comment

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

@nebula-aac @sudhanshutech we dont need to have a complex naming convention , lets agree of something simple and stay consistent

@aabidsofi19
Copy link
Contributor

@sudhanshutech @nebula-aac I would suggest using our custom convention for both private and public components

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

Successfully merging this pull request may close these issues.

3 participants