Skip to content
This repository was archived by the owner on Mar 4, 2020. It is now read-only.

RFC: enhanceName and replaceName to customize styles #1867

Closed
wants to merge 2 commits into from

Conversation

layershifter
Copy link
Member

@layershifter layershifter commented Aug 30, 2019

This PR continues my idea about having styleName name.


⭐️ Goal

  • customize existing components and keep this customizations inside theme
  • use variables from themes as much as possible
  • allow to replace styles at all

❓ How?

Naming is not final and is debatable, however:

  • enchanceName to combine existing styles in a new component, allows to reuse existing variables and introduce new. Should contain the name of original component
  • replaceName to reset all component styles and style components from stratch

These props can't be combined.

🐺 Try it

I updated a Button example, so you can simply try it. Use Popup out mode.

🔨 Unresolved issues?

I don't have ideas about nested components like Menu, Dropdown, etc. 🤔

Pros

  • 👍 it's still Button component, no conflicts with factories
  • 👍 to create Button:Danger I reused existing Button variables and used colorSchema
  • 👍 to create ButtonTertiary I was able to follow existing Stardust patterns and used custom variables and custom styles. Accessibility and all other concerns are still there. And I didn't introduce a new component 🏆
  • 👍 under replaceName I don't need to fight with existing styles
  • 👍 API and changes are dumb
  • 👍 custom styles are scoped and there no conflicts and conditions
  • 👍 theming capabilities are kept

Cons

Post yours!

@DustyTheBot
Copy link
Collaborator

Warnings
⚠️ There are no updates provided to CHANGELOG. Ensure there are no publicly visible changes introduced by this PR.

Generated by 🚫 dangerJS

@codecov
Copy link

codecov bot commented Aug 30, 2019

Codecov Report

Merging #1867 into master will decrease coverage by 0.01%.
The diff coverage is 57.14%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1867      +/-   ##
==========================================
- Coverage   69.74%   69.72%   -0.02%     
==========================================
  Files         885      885              
  Lines        7786     7793       +7     
  Branches     2274     2253      -21     
==========================================
+ Hits         5430     5434       +4     
- Misses       2348     2351       +3     
  Partials        8        8
Impacted Files Coverage Δ
packages/react/src/lib/renderComponent.tsx 81.81% <57.14%> (-2.93%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2d00948...7e03969. Read the comment docs.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
❌ do not merge needs author changes Author needs to implement changes before merge RFC
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants