This repository has been archived by the owner on Mar 4, 2020. It is now read-only.
[WIP] feat: add compose(), caching, converting components to hooks #2229
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Perf comparison
Potential regressions comparing to master
Perf tests with no regressions
Generated by 🚫 dangerJS |
| fluentToFabric <= 0.5 && '⚡', | ||
| fluentToFabric <= 0.7 && '🦄', | ||
| fluentToFabric <= 1 && '🎯', | ||
| '🔧', |
There was a problem hiding this comment.
Our "target" is 30% faster. The previous "on target' was displayed when a Fluent component was 30% faster than Fabric.
The 4 levels are fun, but it might make this harder to understand.
dzearing
reviewed
Jan 22, 2020
| overrideStyles?: boolean | ||
| } | ||
|
|
||
| const COMPOSE_CONFIG_PROP_NAME = '__unstable_config' |
There was a problem hiding this comment.
Should this go in the react-theming package? I thought that was where we were going to have compositional utilities.
We need a package which partners can easily use to compose components and provide a theme, with the least amount of other dependencies in it. This will allow a solid foundation to exist for building out an ecosystem of components.
There was a problem hiding this comment.
I don't think that composition covers only styling aspects 🤔
layershifter
changed the title
layershifter
force-pushed
the
feat/compose
branch
from
26cd53b
to
0521e5a
Compare
layershifter
commented
Jan 24, 2020
| <> | ||
| <Slider /> | ||
| <Slider /> | ||
| </> |
mnajdova
reviewed
Jan 24, 2020
| const renderIcon = () => { | ||
| return Icon.create(icon, { | ||
| // @ts-ignore | ||
| return ButtonIcon.create(icon, { |
There was a problem hiding this comment.
Still not sure about the ButtonIcon component... Maybe in the end we can just combine once the styles for the Icon component + styles for the Button's icon slot and store that as a cache... I am worried that for more complicated component, creating variant for the component will mean creating new variants for each slot, and you may only need to change one style for them...
…dust-ui/react into feat/compose � Conflicts: � packages/state/src/index.ts
…dust-ui/react into feat/compose � Conflicts: � packages/accessibility/src/behaviors/index.ts � packages/react-bindings/src/hooks/useStyles.ts � packages/react-bindings/src/styles/getStyles.ts � packages/react-bindings/src/styles/resolveStylesAndClasses.tsx � packages/react-bindings/test/styles/resolveStylesAndClasses-test.ts � packages/react/src/components/Chat/ChatItem.tsx � packages/react/src/components/Chat/ChatMessage.tsx � packages/react/src/components/Checkbox/Checkbox.tsx � packages/react/src/components/Icon/Icon.tsx � packages/react/src/components/Label/Label.tsx � packages/react/src/components/Slider/Slider.tsx � packages/react/src/components/Status/Status.tsx � packages/react/src/components/Text/Text.tsx � packages/react/src/themes/teams/components/Checkbox/checkboxStyles.ts � packages/react/src/themes/teams/components/Icon/iconStyles.ts � packages/react/src/themes/teams/components/Label/labelStyles.ts � packages/react/src/themes/teams/components/Slider/sliderStyles.ts � packages/react/src/themes/teams/components/Text/textStyles.ts � packages/react/test/specs/components/Checkbox/Checkbox-test.tsx � packages/react/test/specs/components/Icon/Icon-test.tsx � packages/react/test/specs/components/Label/Label-test.tsx � packages/react/test/specs/components/Status/Status-test.tsx � packages/react/test/specs/components/Text/Text-test.tsx � packages/state/src/index.ts
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
DO NOT REVIEW
EARLY DRAFT
NOTICE
This PR covers too many things. We are going to rebase it, extract and discuss things before merging.