-
Notifications
You must be signed in to change notification settings - Fork 4k
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(perf): refactor ComponentControls #3069
Conversation
Signed-off-by: Oleksandr Fediashov <ofediashov@exadel.com>
Codecov Report
@@ Coverage Diff @@
## master #3069 +/- ##
=======================================
Coverage 99.92% 99.92%
=======================================
Files 163 163
Lines 2738 2738
=======================================
Hits 2736 2736
Misses 2 2 Continue to review full report at Codecov.
|
) | ||
|
||
ComponentControlsEditCode.propTypes = { | ||
active: PropTypes.bool, | ||
onClick: PropTypes.func, | ||
} | ||
|
||
export default updateForKeys(['active'])(ComponentControlsEditCode) | ||
export default ComponentControlsEditCode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now, there is no sence in this HOCs.
This is exactly what prompted the refactor here! I was quite surprised and bummed out that people didn't know about the editor :( |
Should we also reverse the order of the links as in Stardust? This makes the first link "Try it". |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See question, make a decision, and merge 👍
On second thought, we can always reorder them after this is merged 😄 |
Before
After
This PR grabs controls from Stardust. I asked my collegues about this, before this change they don't know about editor 🤔
This also allows us to omit
Popup
s creation for each example, it costs over 50-100 ms in development mode.