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

feat(react): remove @charcoal/styled from checkbox #377

Merged
merged 21 commits into from
Dec 13, 2023

Conversation

naporin0624
Copy link
Member

@naporin0624 naporin0624 commented Aug 16, 2023

やったこと

  • @charcoal/styled をはがす
  • selector の最適化

動作確認環境

checkbox styled checkbox remove styled
Image from Gyazo Image from Gyazo

チェックリスト

不要なチェック項目は消して構いません

  • 破壊的変更がある場合には、対象のパッケージのメジャーバージョンが上がっていることを確認した
  • 追加したコンポーネントが index.ts から再 export されている
  • README やドキュメントに影響があることを確認した

@naporin0624 naporin0624 temporarily deployed to preview-channel August 16, 2023 06:49 — with GitHub Actions Inactive
@github-actions
Copy link

github-actions bot commented Aug 16, 2023

Size Change: +1.08 kB (0%)

Total Size: 589 kB

Filename Size Change
./packages/react/dist/components/Checkbox/index.story.d.ts 643 B +61 B (+10%) ⚠️
./packages/react/dist/index.cjs.js 81.4 kB +504 B (+1%)
./packages/react/dist/index.esm.js 72.6 kB +512 B (+1%)
ℹ️ View Unchanged
Filename Size
./packages/foundation/dist/border-radius.d.ts 692 B
./packages/foundation/dist/breakpoint.d.ts 225 B
./packages/foundation/dist/color.d.ts 171 B
./packages/foundation/dist/effect.d.ts 1.08 kB
./packages/foundation/dist/grid.d.ts 396 B
./packages/foundation/dist/index.cjs.js 2.82 kB
./packages/foundation/dist/index.d.ts 230 B
./packages/foundation/dist/index.esm.js 1.44 kB
./packages/foundation/dist/spacing.d.ts 305 B
./packages/foundation/dist/typography.d.ts 790 B
./packages/icons-cli/dist/index.js 20.3 kB
./packages/icons/dist/charcoalIconFiles.d.ts 4.58 kB
./packages/icons/dist/index.cjs.js 9.47 kB
./packages/icons/dist/index.d.ts 478 B
./packages/icons/dist/index.esm.js 7.96 kB
./packages/icons/dist/loaders/CharcoalIconFilesLoader.d.ts 7.25 kB
./packages/icons/dist/loaders/CustomIconLoader.d.ts 417 B
./packages/icons/dist/loaders/index.d.ts 181 B
./packages/icons/dist/loaders/Loadable.d.ts 98 B
./packages/icons/dist/loaders/PixivIconLoadError.d.ts 153 B
./packages/icons/dist/PixivIcon.d.ts 1.46 kB
./packages/icons/dist/PixivIcon.story.d.ts 885 B
./packages/icons/dist/PixivIcon.test.d.ts 55 B
./packages/icons/dist/ssr.d.ts 75 B
./packages/react-sandbox/dist/_lib/compat.d.ts 427 B
./packages/react-sandbox/dist/components/Carousel/index.d.ts 1.35 kB
./packages/react-sandbox/dist/components/Carousel/index.story.d.ts 188 B
./packages/react-sandbox/dist/components/CarouselButton/index.d.ts 619 B
./packages/react-sandbox/dist/components/CarouselButton/index.story.d.ts 323 B
./packages/react-sandbox/dist/components/Filter/index.d.ts 1.12 kB
./packages/react-sandbox/dist/components/Filter/index.story.d.ts 374 B
./packages/react-sandbox/dist/components/HintText/index.d.ts 298 B
./packages/react-sandbox/dist/components/HintText/index.story.d.ts 312 B
./packages/react-sandbox/dist/components/icons/Base.d.ts 706 B
./packages/react-sandbox/dist/components/icons/DotsIcon.d.ts 282 B
./packages/react-sandbox/dist/components/icons/InfoIcon.d.ts 87 B
./packages/react-sandbox/dist/components/icons/NextIcon.d.ts 281 B
./packages/react-sandbox/dist/components/icons/WedgeIcon.d.ts 462 B
./packages/react-sandbox/dist/components/Layout/index.d.ts 2.07 kB
./packages/react-sandbox/dist/components/Layout/index.story.d.ts 402 B
./packages/react-sandbox/dist/components/LeftMenu/index.d.ts 382 B
./packages/react-sandbox/dist/components/MenuListItem/index.d.ts 1.79 kB
./packages/react-sandbox/dist/components/MenuListItem/index.story.d.ts 725 B
./packages/react-sandbox/dist/components/Pager/index.d.ts 529 B
./packages/react-sandbox/dist/components/Pager/index.story.d.ts 1.08 kB
./packages/react-sandbox/dist/components/SwitchCheckbox/index.d.ts 340 B
./packages/react-sandbox/dist/components/SwitchCheckbox/index.story.d.ts 429 B
./packages/react-sandbox/dist/components/TextEllipsis/helper.d.ts 235 B
./packages/react-sandbox/dist/components/TextEllipsis/index.d.ts 381 B
./packages/react-sandbox/dist/components/TextEllipsis/index.story.d.ts 345 B
./packages/react-sandbox/dist/components/WithIcon/index.d.ts 1.07 kB
./packages/react-sandbox/dist/components/WithIcon/index.story.d.ts 609 B
./packages/react-sandbox/dist/foundation/constants.d.ts 208 B
./packages/react-sandbox/dist/foundation/hooks.d.ts 1.04 kB
./packages/react-sandbox/dist/foundation/support.d.ts 131 B
./packages/react-sandbox/dist/foundation/utils.d.ts 613 B
./packages/react-sandbox/dist/hooks/index.d.ts 148 B
./packages/react-sandbox/dist/index.cjs.js 64.5 kB
./packages/react-sandbox/dist/index.d.ts 1.34 kB
./packages/react-sandbox/dist/index.esm.js 55 kB
./packages/react-sandbox/dist/misc/storybook-helper.d.ts 343 B
./packages/react-sandbox/dist/styled.d.ts 12.3 kB
./packages/react/dist/_lib/compat.d.ts 1.44 kB
./packages/react/dist/_lib/index.d.ts 1.02 kB
./packages/react/dist/_lib/useForwardedRef.d.ts 169 B
./packages/react/dist/components/a11y.test.d.ts 50 B
./packages/react/dist/components/Button/index.d.ts 469 B
./packages/react/dist/components/Button/index.story.d.ts 1.26 kB
./packages/react/dist/components/Checkbox/index.d.ts 787 B
./packages/react/dist/components/Checkbox/performance.test.d.ts 252 B
./packages/react/dist/components/Checkbox/snapshot.test.d.ts 76 B
./packages/react/dist/components/Clickable/index.d.ts 747 B
./packages/react/dist/components/Clickable/index.story.d.ts 356 B
./packages/react/dist/components/DropdownSelector/Divider.d.ts 230 B
./packages/react/dist/components/DropdownSelector/DropdownMenuItem.d.ts 308 B
./packages/react/dist/components/DropdownSelector/DropdownPopover.d.ts 486 B
./packages/react/dist/components/DropdownSelector/index.d.ts 555 B
./packages/react/dist/components/DropdownSelector/index.story.d.ts 632 B
./packages/react/dist/components/DropdownSelector/ListItem/index.d.ts 668 B
./packages/react/dist/components/DropdownSelector/ListItem/index.story.d.ts 287 B
./packages/react/dist/components/DropdownSelector/MenuItem/index.d.ts 511 B
./packages/react/dist/components/DropdownSelector/MenuItem/internals/handleFocusByKeyBoard.d.ts 373 B
./packages/react/dist/components/DropdownSelector/MenuItem/internals/useMenuItemHandleKeyDown.d.ts 483 B
./packages/react/dist/components/DropdownSelector/MenuItemGroup/index.d.ts 476 B
./packages/react/dist/components/DropdownSelector/MenuList/index.d.ts 699 B
./packages/react/dist/components/DropdownSelector/MenuList/index.story.d.ts 390 B
./packages/react/dist/components/DropdownSelector/MenuList/internals/getValuesRecursive.d.ts 348 B
./packages/react/dist/components/DropdownSelector/MenuList/MenuListContext.d.ts 325 B
./packages/react/dist/components/DropdownSelector/Popover/index.d.ts 632 B
./packages/react/dist/components/DropdownSelector/Popover/index.story.d.ts 283 B
./packages/react/dist/components/DropdownSelector/Popover/usePreventScroll.d.ts 140 B
./packages/react/dist/components/DropdownSelector/utils/findPreviewRecursive.d.ts 411 B
./packages/react/dist/components/FieldLabel/index.d.ts 470 B
./packages/react/dist/components/Icon/index.d.ts 498 B
./packages/react/dist/components/Icon/index.story.d.ts 5 kB
./packages/react/dist/components/IconButton/index.d.ts 572 B
./packages/react/dist/components/IconButton/index.story.d.ts 811 B
./packages/react/dist/components/LoadingSpinner/index.d.ts 658 B
./packages/react/dist/components/LoadingSpinner/index.story.d.ts 404 B
./packages/react/dist/components/Modal/__stories__/InternalScrollStory.d.ts 189 B
./packages/react/dist/components/Modal/Dialog/index.d.ts 1.6 kB
./packages/react/dist/components/Modal/index.d.ts 1.94 kB
./packages/react/dist/components/Modal/index.story.d.ts 1.5 kB
./packages/react/dist/components/Modal/ModalBackgroundContext.d.ts 231 B
./packages/react/dist/components/Modal/ModalPlumbing.d.ts 510 B
./packages/react/dist/components/MultiSelect/context.d.ts 394 B
./packages/react/dist/components/MultiSelect/index.d.ts 1.2 kB
./packages/react/dist/components/MultiSelect/index.story.d.ts 2 kB
./packages/react/dist/components/MultiSelect/index.test.d.ts 51 B
./packages/react/dist/components/Radio/index.d.ts 1.07 kB
./packages/react/dist/components/Radio/index.story.d.ts 1.01 kB
./packages/react/dist/components/Radio/index.test.d.ts 51 B
./packages/react/dist/components/SegmentedControl/index.d.ts 736 B
./packages/react/dist/components/SegmentedControl/index.story.d.ts 500 B
./packages/react/dist/components/SegmentedControl/RadioGroupContext.d.ts 350 B
./packages/react/dist/components/Switch/index.d.ts 512 B
./packages/react/dist/components/Switch/index.story.d.ts 531 B
./packages/react/dist/components/TagItem/index.d.ts 1.2 kB
./packages/react/dist/components/TagItem/index.story.d.ts 1.14 kB
./packages/react/dist/components/TextArea/index.d.ts 1.09 kB
./packages/react/dist/components/TextArea/TextArea.story.d.ts 1.01 kB
./packages/react/dist/components/TextField/index.d.ts 1.53 kB
./packages/react/dist/components/TextField/TextField.story.d.ts 1.08 kB
./packages/react/dist/components/TextField/useFocusWithClick.d.ts 242 B
./packages/react/dist/core/CharcoalProvider.d.ts 656 B
./packages/react/dist/core/ComponentAbstraction.d.ts 933 B
./packages/react/dist/core/OverlayProvider.d.ts 101 B
./packages/react/dist/core/SSRProvider.d.ts 88 B
./packages/react/dist/index.d.ts 2.05 kB
./packages/react/dist/styled.d.ts 12.3 kB
./packages/styled/dist/builders/border.d.ts 685 B
./packages/styled/dist/builders/borderRadius.d.ts 440 B
./packages/styled/dist/builders/colors.d.ts 1.3 kB
./packages/styled/dist/builders/elementEffect.d.ts 533 B
./packages/styled/dist/builders/o.d.ts 5.91 kB
./packages/styled/dist/builders/outline.d.ts 638 B
./packages/styled/dist/builders/size.d.ts 1.19 kB
./packages/styled/dist/builders/spacing.d.ts 1.17 kB
./packages/styled/dist/builders/transition.d.ts 287 B
./packages/styled/dist/builders/typography.d.ts 624 B
./packages/styled/dist/defineThemeVariables.test.d.ts 66 B
./packages/styled/dist/factories/lib.d.ts 3.96 kB
./packages/styled/dist/helper.d.ts 2.01 kB
./packages/styled/dist/index.cjs.js 26.7 kB
./packages/styled/dist/index.d.ts 6.24 kB
./packages/styled/dist/index.esm.js 24.2 kB
./packages/styled/dist/index.story.d.ts 561 B
./packages/styled/dist/index.test.d.ts 73 B
./packages/styled/dist/internals/index.d.ts 1.6 kB
./packages/styled/dist/SetThemeScript.d.ts 905 B
./packages/styled/dist/storyHelper.d.ts 299 B
./packages/styled/dist/TokenInjector.d.ts 505 B
./packages/styled/dist/util.d.ts 4.24 kB
./packages/tailwind-config/dist/_lib/compat.d.ts 418 B
./packages/tailwind-config/dist/_lib/TailwindBuild.d.ts 750 B
./packages/tailwind-config/dist/colors/plugin.d.ts 443 B
./packages/tailwind-config/dist/colors/plugin.test.d.ts 52 B
./packages/tailwind-config/dist/colors/toTailwindConfig.d.ts 394 B
./packages/tailwind-config/dist/colors/toTailwindConfig.test.d.ts 62 B
./packages/tailwind-config/dist/colors/utils.d.ts 347 B
./packages/tailwind-config/dist/docs/borderRadius/BorderRadius.d.ts 119 B
./packages/tailwind-config/dist/docs/borderRadius/index.d.ts 175 B
./packages/tailwind-config/dist/docs/colors/Colors.d.ts 107 B
./packages/tailwind-config/dist/docs/colors/index.d.ts 201 B
./packages/tailwind-config/dist/docs/colors/TextBgColor.d.ts 175 B
./packages/tailwind-config/dist/docs/colors/TextBgColor.story.d.ts 298 B
./packages/tailwind-config/dist/docs/colors/TextColors.d.ts 115 B
./packages/tailwind-config/dist/docs/gradient/Gradients.d.ts 113 B
./packages/tailwind-config/dist/docs/gradient/index.d.ts 433 B
./packages/tailwind-config/dist/docs/gradient/utils.d.ts 120 B
./packages/tailwind-config/dist/docs/index.d.ts 360 B
./packages/tailwind-config/dist/docs/screens/index.d.ts 160 B
./packages/tailwind-config/dist/docs/screens/Screens.d.ts 109 B
./packages/tailwind-config/dist/docs/spacing/index.d.ts 160 B
./packages/tailwind-config/dist/docs/spacing/Spacing.d.ts 109 B
./packages/tailwind-config/dist/docs/typography/HalfLeading.d.ts 117 B
./packages/tailwind-config/dist/docs/typography/index.d.ts 346 B
./packages/tailwind-config/dist/docs/typography/Sizes.d.ts 105 B
./packages/tailwind-config/dist/foundation.d.ts 360 B
./packages/tailwind-config/dist/gradient/plugin.d.ts 816 B
./packages/tailwind-config/dist/gradient/plugin.test.d.ts 52 B
./packages/tailwind-config/dist/index.cjs.js 11.5 kB
./packages/tailwind-config/dist/index.d.ts 373 B
./packages/tailwind-config/dist/index.esm.js 9.6 kB
./packages/tailwind-config/dist/index.test.d.ts 51 B
./packages/tailwind-config/dist/types.d.ts 855 B
./packages/tailwind-config/dist/typography/plugin.d.ts 146 B
./packages/tailwind-config/dist/util.d.ts 656 B
./packages/tailwind-diff/dist/commands/check.d.ts 296 B
./packages/tailwind-diff/dist/commands/check.js 4.37 kB
./packages/tailwind-diff/dist/commands/dump.d.ts 185 B
./packages/tailwind-diff/dist/commands/dump.js 1.26 kB
./packages/tailwind-diff/dist/defer.d.ts 164 B
./packages/tailwind-diff/dist/defer.js 815 B
./packages/tailwind-diff/dist/index.d.ts 46 B
./packages/tailwind-diff/dist/index.js 1.84 kB
./packages/tailwind-diff/dist/packageManager.d.ts 323 B
./packages/tailwind-diff/dist/packageManager.js 1.85 kB
./packages/tailwind-diff/dist/style.d.ts 436 B
./packages/tailwind-diff/dist/style.js 2.68 kB
./packages/tailwind-diff/dist/withPackages.d.ts 464 B
./packages/tailwind-diff/dist/withPackages.js 3.24 kB
./packages/theme/dist/abstract-theme.d.ts 1.19 kB
./packages/theme/dist/default.d.ts 163 B
./packages/theme/dist/index.cjs.js 5.17 kB
./packages/theme/dist/index.d.ts 121 B
./packages/theme/dist/index.esm.js 3.48 kB
./packages/theme/dist/theme.d.ts 2.87 kB
./packages/utils/dist/index.cjs.js 6.24 kB
./packages/utils/dist/index.d.ts 3.17 kB
./packages/utils/dist/index.esm.js 4.51 kB

compressed-size-action

@github-actions
Copy link

github-actions bot commented Aug 16, 2023

Visit the preview URL for this PR (updated for commit 0036b15):

https://pixiv-charcoal-web--pr377-feat-remove-charcoal-mquzct3q.web.app

(expires Wed, 06 Dec 2023 07:02:32 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 314b26d3adca98a761c7e4d9922ebb206ff024a0

@naporin0624 naporin0624 force-pushed the feat/remove-charcoal-style-from-checkbox branch from a1d83c5 to 0412113 Compare August 16, 2023 06:55
@naporin0624 naporin0624 temporarily deployed to preview-channel August 16, 2023 06:55 — with GitHub Actions Inactive
@naporin0624 naporin0624 temporarily deployed to preview-channel August 16, 2023 07:00 — with GitHub Actions Inactive
@@ -263,8 +261,6 @@ exports[`using dark theme storiesOf(index.story.tsx).add(Unlabelled) snapshot 1`
width: 20px;
height: 20px;
border-radius: 4px;
-webkit-transition: 0.2s box-shadow;
transition: 0.2s box-shadow;
-webkit-transition: all 0.2s !important;
transition: all 0.2s !important;
Copy link
Member Author

Choose a reason for hiding this comment

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

重複してるので消した

@naporin0624 naporin0624 temporarily deployed to preview-channel August 16, 2023 07:31 — with GitHub Actions Inactive
@naporin0624 naporin0624 changed the title feat(react): replace static css feat(react): remove @charcoal/styled from checkbox Aug 16, 2023
@naporin0624 naporin0624 temporarily deployed to preview-channel August 16, 2023 08:57 — with GitHub Actions Inactive
.c2[type='checkbox']:not(:checked) {
border-width: 2px;
border-style: solid;
border-color: #5c5c5c;
Copy link
Member Author

Choose a reason for hiding this comment

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

ここが直接 hex color で出ていた

&:not(:checked) {
border-width: 2px;
border-style: solid;
border-color: ${({ theme }) => theme.color.text4};
Copy link
Member Author

Choose a reason for hiding this comment

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

theme.color.text4 が使われているとき hex color で出力されたので css variables に書き直してる

"@react-aria/dialog": "^3.5.0",
"@react-aria/focus": "^3.11.0",
"@react-aria/listbox": "^3.8.0",
"@react-aria/overlays": "^3.13.0",
"@react-aria/radio": "^3.5.0",
"@react-aria/radio": "^3.8.0",
Copy link
Member Author

Choose a reason for hiding this comment

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

react-stately を上げたことで型の整合が取れなくなったので version up した

Comment on lines 28 to 32
invalid: {
description: '入力の不正化',
default: 'false',
type: 'boolean',
},
Copy link
Member Author

@naporin0624 naporin0624 Sep 27, 2023

Choose a reason for hiding this comment

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

ci の build が落ちるかつ docs に足りてなかったので記述を追加

@@ -60,6 +60,30 @@ Labelled.args = {
invalid: false,
}

export const Invalid: Story<Omit<Props, 'invalid'>> = (props) => {
Copy link
Member Author

Choose a reason for hiding this comment

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

snapshot を生成するために Invalid の story を追加

@@ -41,6 +41,7 @@
"@charcoal-ui/tailwind-config": "../packages/tailwind-config",
"@charcoal-ui/tailwind-diff": "../packages/tailwind-diff",
"@charcoal-ui/theme": "../packages/theme",
"@charcoal-ui/utils": "../packages/utils"
"@charcoal-ui/utils": "../packages/utils",
"next/@swc/helpers": "0.4.36"
Copy link
Member Author

Choose a reason for hiding this comment

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

react-aria の version up をしたところ docs の next build が通らなくなったので issue の内容で修正
vercel/next.js#48593 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

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

これ charcoal を使用している next.js のプロダクトすべてに影響を及ぼす気がするんですが、react-aria の version up は避けたほうがいいですかね......

Comment on lines +138 to +139
&[aria-invalid='true'] {
box-shadow: 0 0 0 4px rgba(255, 43, 0, 0.32);
Copy link
Member Author

Choose a reason for hiding this comment

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

approve あと追加分

react-aria から checkbox に aria-invalid が張られるようになったので動的に切り替える方法から selector に統一

@naporin0624 naporin0624 self-assigned this Sep 27, 2023
@naporin0624 naporin0624 merged commit 67529dd into main Dec 13, 2023
10 checks passed
@naporin0624 naporin0624 deleted the feat/remove-charcoal-style-from-checkbox branch December 13, 2023 07:05
@toshusai toshusai mentioned this pull request Mar 5, 2024
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants