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

Add carbon and theming to core package #3

Closed
ericabr opened this issue Jun 25, 2020 · 7 comments
Closed

Add carbon and theming to core package #3

ericabr opened this issue Jun 25, 2020 · 7 comments
Labels
status: won’t fix 🔚 This will not be worked on type: enhancement ✨ New feature or request

Comments

@ericabr
Copy link

ericabr commented Jun 25, 2020

Core package:

@ericabr ericabr added the type: enhancement ✨ New feature or request label Jun 25, 2020
@SimonFinney
Copy link
Contributor

SimonFinney commented Jul 8, 2020

Very cool! Another thing we could consider is adding a theme switcher to the Storybook to enable switching between dark and light themes - @carbon-design-system/ibm-cloud-paks thoughts on this and what the default theme might be?

@ericabr
Copy link
Author

ericabr commented Jul 8, 2020

Yes, I love that Idea! I think the default theme should be the dark theme since it looks like we are headed that way.

@matthew-chirgwin
Copy link

Hi! I missed the last interlock (I will be able join today however), but is there a description of all the packages, and what the architecture/relationship they have to each other I could catch up on? The reason I ask (and I more than likely have the wrong end of the stick!) is if all packages consume core, that would default everyone to the same version of Carbon/the theme used say. Could that cause issues (one package wanting version X, others wanting Y for example)?

@ericabr
Copy link
Author

ericabr commented Jul 8, 2020

I think for now, the other packages don't have to consume core. In the long run, we want all of the other packages to consume core once we are more aligned. Core will be the driving force of the version of carbon we should all try and keep in line with once we actually start pointing towards it.
Today, the packages being hosted here are:

  • Automation
  • CD & AI
  • Security
  • Bedrock services ->common services

I think we should add these to the common README for others awareness.

@matthew-chirgwin
Copy link

Now I have caught up, just wanted to share my thoughts on what I would suggest a core package could do going forward:

  • I see a core package being mainly helpful utility pieces - common test helpers, build configuration, linting, common storybook pieces and so on
    • With the intent that teams migrate to using these helpers and so on in their current code - in turn, meaning we all converge on a common style/approach
    • To aid this convergence, documentation around approach, rules of engagement and best practise should be written to aid conversion, as well as guide new implementation
  • It should contain useful/reusable assets - icons, images, etc
  • Rather than depending/exporting carbon - it defines/exports the version of carbon (carbon-components-react, carbon-icons, carbon-components) we all ‘should’ use in our own package.json (expected to be spread say into the dependancy section)
    • Allows flexibility for consumers to use alternative versions when required - we will have cases where package A needs a fix, but B doesn’t want to change due to a code freeze etc

Theming is an interesting bit. In my experience in the CDAI package, we have had a number of discussions (and contention) around having a set theme that all consumers must use. In our case the designs given to engineering did not account for some of the technical imitations around Carbon components and their theme-ability. This happens typically (but was not limited to) where we need Carbon component X in a light variant, but such a thing does not exist.

This is one of the many examples where deviation from Carbon will be required in its current form (and one of the reasons I err when suggesting set values in a core package - there will be cases where this will not work for all).

What I would suggest we do (and again, documentation in the core package could guide/define this) is full use of Carbon tokens and mixins for colour, typography, spacing and layout etc in all styling. This means if overrides are needed (if any), they are in this case theme agnostic, and consumable (if needed) by everyone with minimal changes. These overrides should be contributed to the core package for others to use if needed - if we need to diviate, we all should do so using the same code. By doing this, the setting of the theme can then be the responsibility of the consumer of the package (ie CDAI for instance). By doing this, theming then isn’t a responsibility of the core package, but as a group of packages, we can support any theme.

@lee-chase
Copy link
Member

Raised carbon-design-system/carbon#6828

@stale
Copy link

stale bot commented Nov 14, 2020

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the wontfix label Nov 14, 2020
@SimonFinney SimonFinney added the status: won’t fix 🔚 This will not be worked on label Nov 14, 2020
@stale stale bot removed the wontfix label Nov 14, 2020
@stale stale bot closed this as completed Nov 21, 2020
@sydrosa sydrosa mentioned this issue Mar 31, 2021
15 tasks
kodiakhq bot added a commit that referenced this issue Mar 23, 2023
* build: test sync workflow

* chore: update formatting

* chore: update indentation

* chore: update branch for testing

* build: test sync workflow (#2)

* build: test sync workflow

* chore: update formatting

* chore: update indentation

* chore: update branch for testing

* build: checkout commit hash

* chore: merge `sync-commit` into `cherry-pick-changes`

* build: test sync workflow (#3)

* build: test sync workflow

* chore: update formatting

* chore: update indentation

* chore: update branch for testing

* build: checkout commit hash

* chore: merge `sync-commit` into `cherry-pick-changes`

* chore: test cherry-pick

* build: test sync workflow (#4)

* build: test sync workflow

* chore: update formatting

* chore: update indentation

* chore: update branch for testing

* build: checkout commit hash

* chore: merge `sync-commit` into `cherry-pick-changes`

* chore: test cherry-pick

* chore: add `test.md` (#5)

* chore: test fetch

* chore: add Git configuration

* build: update PR body

* build: update PR body

* chore: update issue configuration

* chore: fix formatting

* build: add labels to PR

* build: update PR formatting

* build: update merge strategy

* build: use commit author

* Revert "build: use commit author"

This reverts commit e9e9d4a.

* chore: update for PR submission

---------

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: won’t fix 🔚 This will not be worked on type: enhancement ✨ New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants