[Joy] Add `Button` - 1st iteration #29464

siriwatknp · 2021-11-02T08:26:42Z

https://deploy-preview-29464--material-ui.netlify.app/joy/

This PR

please review the implementation & coding pattern, so that we can make this as a standard for other Joy components.

Props
- variant: text | outlined | contained (same as material)
- size: small | medium | large (same as material)
- color: brand (related to theme.palette)
- fullWidth
classes (global className are not combined)
- no classes as prop

The design is not final, please skip the design discussion in this PR.

Next iteration

Design
Props
- start & end icon (with SvgIcon component)
- loading
- icon button like
- ButtonGroup

I have followed (at least) the PR section of the contributing guide.

This reverts commit 4e5f739.

This reverts commit ae8d466.

mbrookes · 2021-11-12T15:43:10Z

color: brand (related to theme.palette)

Please see my feedback in Notion

siriwatknp · 2021-11-15T06:48:11Z

color: brand (related to theme.palette)

Please see my feedback in Notion

FYI, this PR is not about design, I will fix it once we have decided about the naming but it should not block this PR.

mbrookes · 2021-11-15T21:35:01Z

Please see my feedback in Notion

FYI, this PR is not about design

That's why I left the feedback in Notion not here. 😄 (Not that anyone has responded to it...)
This was simply what prompted the thought.

…joy/button

packages/mui-joy/src/Button/Button.test.js

mnajdova · 2021-11-24T11:15:43Z

packages/mui-joy/src/Button/Button.test.js

+    skip: ['propsSpread', 'componentsProp', 'classesRoot'],
+  }));
+
+  it('by default, should render with the root, variantText, sizeMedium and colorBrand classes', () => {


Based on the tests if the button is rendered with all classes all test would pass :) Shouldw e maybe test that by default the other classes are not added? Or update the other test to test that the class is not there by default, but it is if the props is added.

Shouldw e maybe test that by default the other classes are not added

Added.

packages/mui-joy/src/Button/Button.tsx

packages/mui-joy/src/styles/components.d.ts

yarn.lock

…joy/button

mnajdova

Looks good 👍

oliviertassinari

Joy is alive :D https://codesandbox.io/s/determined-jepsen-w79jn?file=/src/App.js

packages/mui-joy/src/Button/Button.tsx

oliviertassinari · 2021-11-29T14:10:54Z

packages/mui-joy/src/Button/Button.tsx

+    ];
+  },
+})<{ ownerState: ButtonProps }>(({ theme, ownerState }) => {
+  const colorPalette = theme.vars.palette[ownerState.color || 'brand'];


From a DX perspective, I wonder if we shouldn't reverse the tradeoff. It seems that the right path (using the CSS variable) is more painful (more to type/read) than accessing the raw value.

Suggested change

const colorPalette = theme.vars.palette[ownerState.color || 'brand'];

const colorPalette = theme.palette[ownerState.color || 'brand'];

This is what I propose in #27651 under "Decision on the API".

2 things about using vars

I consider CSS variables as a feature on top of ThemeProvider, so adding vars instead of replace the theme makes more sense.

When we have to update mui-material to support CSS variables, we can incrementally change the code in each component and it should not break other components.

Anyway, I will try your method and share the finding again. If it is better, we can change Joy to use it.

oliviertassinari · 2021-11-29T14:22:48Z

test/utils/describeConformance.js

+        classes,
+        testStateOverrides,
+        render,
+        ThemeProvider = MDThemeProvider,


Did we settle on the naming conversions? How are we abbreviating Material Design as "MUI" or "MD"? It seems to be MD and MUI for Material to build UIs. Alright.

MD is better because the option we want to move forward on the docs says "MUI Core as a product line — System, Joy, MD, and Base as products"

siriwatknp added 30 commits October 19, 2021 11:24

add useTheme to createStyled

4e5f739

add default theme to Joy

ef44ca1

update tsconfig to include mui-system

fc8a3e4

add ThemeProvider test

fa484be

Revert "add useTheme to createStyled"

ae8d466

This reverts commit 4e5f739.

Revert "Revert "add useTheme to createStyled""

ddc35f9

This reverts commit ae8d466.

add Partial to style matchers

5b54132

add tests

7c75d62

add more tests

06f336a

rename types

96c7a17

refactor to vars

d1c45af

add tests for createStyled

a83d6c7

remove unused colors

1f1a4eb

rename test

b7cf8dc

fix types augmentation

a5d654a

fix tests

ef574b4

use initial tokens

3745b44

update codesandbox ci to include joy

e458758

create joy example template

83349bb

export dummy Button

ce26656

mark joy as public

63affc8

run propTypes and export ButtonProps

67cadd1

remove non exist path

b88879f

fix README

3fd77ec

add comment to README

2c0bd62

replace dev with alpha

069dcd0

revert createStyled

105c6c8

use ThemeContext from styled-engine

007121a

Merge branch 'joy/css-vars-provider' into joy/button

922cfd3

exclude mui-joy

2a9aa2b

mbrookes added the package: joy-ui Specific to @mui/joy label Nov 12, 2021

siriwatknp added 2 commits November 15, 2021 14:20

fix test

4485b6f

use MuiButton

cc9e3a8

siriwatknp requested review from mnajdova, michaldudak and oliviertassinari November 15, 2021 13:00

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 16, 2021

Merge branch 'master' into joy/button

c18f897

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 17, 2021

bump deps

83d1981

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 19, 2021

Merge branch 'master' of https://github.com/mui-org/material-ui into …

fdcf907

…joy/button

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 24, 2021

remove temporary page

fa80081

mnajdova reviewed Nov 24, 2021

View reviewed changes

siriwatknp added 3 commits November 25, 2021 08:23

revert yarn.lock

eefe522

exclude GlobalStateSlot on styleOverrides

3e32c2f

fix test

db055d4

siriwatknp requested a review from mnajdova November 25, 2021 02:01

fix types

77fb0f0

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 25, 2021

siriwatknp added 2 commits November 29, 2021 16:35

Merge branch 'master' of https://github.com/mui-org/material-ui into …

834d2e1

…joy/button

remove unnecessary shouldForwardprop

b87df29

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 29, 2021

mnajdova approved these changes Nov 29, 2021

View reviewed changes

siriwatknp merged commit 56c9e5e into mui:master Nov 29, 2021

oliviertassinari reviewed Nov 29, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Joy] Add `Button` - 1st iteration #29464

[Joy] Add `Button` - 1st iteration #29464

siriwatknp commented Nov 2, 2021 •

edited

Loading

mbrookes commented Nov 12, 2021

siriwatknp commented Nov 15, 2021

mbrookes commented Nov 15, 2021

mnajdova Nov 24, 2021

siriwatknp Nov 25, 2021

mnajdova left a comment

oliviertassinari left a comment

oliviertassinari Nov 29, 2021 •

edited

Loading

siriwatknp Nov 30, 2021

oliviertassinari Nov 29, 2021 •

edited

Loading

siriwatknp Nov 30, 2021 •

edited

Loading

	const colorPalette = theme.vars.palette[ownerState.color \|\| 'brand'];
	const colorPalette = theme.palette[ownerState.color \|\| 'brand'];

[Joy] Add Button - 1st iteration #29464

[Joy] Add Button - 1st iteration #29464

Conversation

siriwatknp commented Nov 2, 2021 • edited Loading

This PR

Next iteration

mbrookes commented Nov 12, 2021

siriwatknp commented Nov 15, 2021

mbrookes commented Nov 15, 2021

mnajdova Nov 24, 2021

Choose a reason for hiding this comment

siriwatknp Nov 25, 2021

Choose a reason for hiding this comment

mnajdova left a comment

Choose a reason for hiding this comment

oliviertassinari left a comment

Choose a reason for hiding this comment

oliviertassinari Nov 29, 2021 • edited Loading

Choose a reason for hiding this comment

siriwatknp Nov 30, 2021

Choose a reason for hiding this comment

oliviertassinari Nov 29, 2021 • edited Loading

Choose a reason for hiding this comment

siriwatknp Nov 30, 2021 • edited Loading

Choose a reason for hiding this comment

[Joy] Add `Button` - 1st iteration #29464

[Joy] Add `Button` - 1st iteration #29464

siriwatknp commented Nov 2, 2021 •

edited

Loading

oliviertassinari Nov 29, 2021 •

edited

Loading

oliviertassinari Nov 29, 2021 •

edited

Loading

siriwatknp Nov 30, 2021 •

edited

Loading