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

[docs] Clarify the difference between React and UI components #29930

Merged
merged 1 commit into from
Jan 18, 2022

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Nov 28, 2021

Should an issue/PR that touches on the <TreeItem> React component have the component: TreeView GitHub label?

While I believe the answer should be yes, so we can have sub-product owners that can monitor the labels they care about, I believe the label casing, colors, descriptions were not making it clear. This change is meant to solve this confusion. Now, we have:

  • GitHub labels that are about UI components. They could include Joy, Material Design, Unstyled, etc. The important part here is that no matter how it's implemented, the end-user experience is what ultimately matters the most (see the survey where people ranked "look & feel" as the number one criteria for picking a UI library). Therefore, I think that we should organize ourselves around it.

    Before
    Screenshot 2021-11-28 at 20 02 01

    After
    Screenshot 2021-11-28 at 20 01 32

  • GitHub labels that are about technical components (React) now have a slightly different description:

    Before
    Screenshot 2021-11-28 at 20 23 20

    After
    Screenshot 2021-11-28 at 20 23 36

I hope this proposal makes it clearer.

@oliviertassinari oliviertassinari added the docs Improvements or additions to the documentation label Nov 28, 2021
@mui-pr-bot
Copy link

mui-pr-bot commented Nov 28, 2021

No bundle size changes

Generated by 🚫 dangerJS against 0f996c1

@mbrookes mbrookes changed the title [docs] Clear the difference between UI and React components [docs] Clarify the difference between React and UI components Nov 28, 2021
@oliviertassinari oliviertassinari requested a review from a team November 29, 2021 14:03
@mnajdova
Copy link
Member

If the issue is about the TreeItem component does that mean that we need to label it with: component: tree view and component: TreeItem, or only the first?

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Nov 30, 2021

If the issue is about the TreeItem component does that mean that we need to label it with: component: tree view and component: TreeItem, or only the first?

@mnajdova I think that the component: tree view label is what would matter the most. The community can then use it to find all the work history on the tree view. The maintainers owning this component can use it to drive all the ongoing effort.

As for adding the component: TreeItem GitHub label, I personally can't think of a use case for it. Do you have one in mind?

Actually, this makes me think of another topic. I believe the in the issue/PR titles [label] Description, what we currently do is put in the label is either be the name of the JS module (e.g. [TreeItem] Fix text selection), or the most important GitHub label (e.g. [tree view] Fix text selection). Usually, what I have noticed we do, is put the JS module when possible, or the GitHub label when it touches multiple JS modules, like <TreeItem> and <TreeView> at the same time.

@oliviertassinari oliviertassinari requested a review from a team November 30, 2021 18:41
@siriwatknp
Copy link
Member

I hope there is no impact on the existing labeled issues. No objection from my side.

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Dec 1, 2021

I hope there is no impact on the existing labeled issues.

@siriwatknp None I'm aware of.

@hbjORbj
Copy link
Member

hbjORbj commented Dec 1, 2021

No objection from my side as well.

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Dec 1, 2021
@mnajdova
Copy link
Member

mnajdova commented Dec 2, 2021

👍 for issues
👍 for using the JS module on PRs (it's more descriptive related to the changes)

Copy link
Member

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

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

Probably needs to be updated once more before merging

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Dec 2, 2021
@m4theushw
Copy link
Member

We had a small discussion about this in mui/mui-x#3312 (comment). Basically, in MUI X we were using the component name for things that users can perceive and "core" for internal stuff of the component. Removing the concept of the component name and using the sub-product name makes more sense and scalable.

Actually, this makes me think of another topic. I believe the in the issue/PR titles [label] Description, what we currently do is put in the label is either be the name of the JS module (e.g. [TreeItem] Fix text selection), or the most important GitHub label (e.g. [tree view] Fix text selection). Usually, what I have noticed we do, is put the JS module when possible, or the GitHub label when it touches multiple JS modules, like and at the same time.

With the new label, does it mean that the title of an issue touching several parts of the TreeView will be prefixed with [tree view]?

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Dec 4, 2021
@mnajdova
Copy link
Member

mnajdova commented Dec 6, 2021

With the new label, does it mean that the title of an issue touching several parts of the TreeView will be prefixed with [tree view]?

As far as I understood not, it should be the JS module, so [TreeView] I suppose,

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Dec 13, 2021
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Dec 22, 2021
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jan 18, 2022
@oliviertassinari oliviertassinari added the new feature New feature or request label Jan 18, 2022
@mui-bot
Copy link

mui-bot commented Jan 18, 2022

No bundle size changes

Generated by 🚫 dangerJS against 7e8542c

@oliviertassinari oliviertassinari merged commit 9ee3234 into mui:master Jan 18, 2022
@oliviertassinari oliviertassinari deleted the update-badges branch January 18, 2022 23:18
wladimirguerra pushed a commit to wladimirguerra/material-ui that referenced this pull request Feb 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation new feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants