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: convert to whitelabel #3616

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

feat: convert to whitelabel #3616

wants to merge 14 commits into from

Conversation

nmerget
Copy link
Member

@nmerget nmerget commented Dec 19, 2024

Proposed changes

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactoring (fix on existing components or architectural decisions)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Update (if none of the other choices apply)

Further comments

@github-actions github-actions bot added 📕documentation Improvements or additions to documentation 🏗foundations Changes inside foundations folder 🏘components Changes inside components folder labels Dec 19, 2024
Copy link
Contributor

🔭🐙🐈 Test this branch here: https://db-ui.github.io/mono/review/feat-whitelabel

@github-actions github-actions bot added the 📺showcases Changes to 1-n showcases label Dec 20, 2024
nmerget added 10 commits January 2, 2025 09:14
# Conflicts:
#	__snapshots__/foundations/chromium/Colors-should-match-screenshot-1/Colors-should-match-screenshot.png
#	__snapshots__/foundations/chromium/Fonts-should-match-screenshot-1/Fonts-should-match-screenshot.png
#	__snapshots__/foundations/chromium/Icons-should-match-screenshot-1/Icons-should-match-screenshot.png
#	__snapshots__/foundations/firefox/Colors-should-match-screenshot-1/Colors-should-match-screenshot.png
#	__snapshots__/foundations/firefox/Fonts-should-match-screenshot-1/Fonts-should-match-screenshot.png
#	__snapshots__/foundations/firefox/Icons-should-match-screenshot-1/Icons-should-match-screenshot.png
#	__snapshots__/foundations/webkit/Colors-should-match-screenshot-1/Colors-should-match-screenshot.png
#	__snapshots__/foundations/webkit/Fonts-should-match-screenshot-1/Fonts-should-match-screenshot.png
#	__snapshots__/foundations/webkit/Icons-should-match-screenshot-1/Icons-should-match-screenshot.png
#	__snapshots__/input/patternhub/input-docs-should-match-screenshot.png
#	__snapshots__/input/showcase/webkit/DBInput-should-match-screenshot-1/DBInput-should-match-screenshot.png
@nmerget nmerget marked this pull request as ready for review January 6, 2025 10:41
@nmerget nmerget requested a review from mfranzke January 6, 2025 10:41
@@ -9,7 +9,7 @@
| 🔄 renamed `Tonality` to `Density` | class names and data-attributes changed from <br/>`.db-ui-#{$tonality},[data-tonality="#{$tonality}"] {` to <br/>`.db-#{density},[data-density="#{density}"] {` | search `tonality` & replace with `density` |
| ❌ removed `opacity` tokens | we use only 1 opacity (0.4) for all components | If you use some of the tokens like `--db-opacity-sm` you might run into issues with your layout |
| 🔄 updated `border` tokens | we add all shirt-sizes `3xs`-`3xl` as tokens | If you use some of the tokens like `db-border-height-sm` you might run into issues with your layout, because the values behind it changed |
| 🔄 moved `_font-sizes.scss` | We moved the file to another folder to align the same structure as icons or colors. We add `css` classes, you can use them by importing `@db-ui/foundations/scss/fonts/classes/all.css` | If you use some placeholder like `%db-overwrite-font-size-sm` you might need to import the `_font-sizes.scss` like this: `@use "@db-ui/foundations/build/scss/fonts";` |
| 🔄 moved `_font-sizes.scss` | We moved the file to another folder to align the same structure as icons or colors. We add `css` classes, you can use them by importing `@db-ui/foundations/scss/fonts/classes/all.css` | If you use some placeholder like `%db-overwrite-font-size-sm` you might need to import the `_font-sizes.scss` like this: `@use "@db-ui/foundations/build/styles/fonts";` |
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
| 🔄 moved `_font-sizes.scss` | We moved the file to another folder to align the same structure as icons or colors. We add `css` classes, you can use them by importing `@db-ui/foundations/scss/fonts/classes/all.css` | If you use some placeholder like `%db-overwrite-font-size-sm` you might need to import the `_font-sizes.scss` like this: `@use "@db-ui/foundations/build/styles/fonts";` |
| 🔄 moved `_font-sizes.scss` | We moved the file to another folder to align the same structure as icons or colors. We add `css` classes, you can use them by importing `@db-ui/foundations/scss/fonts/classes/all.css` | If you use some placeholder like `%db-overwrite-font-size-sm` you might need to import the `_font-sizes.scss` like this: `@use "@db-ui/foundations/build/scss/fonts";` |

At the time of writing (this file), this has been the correct path.

@mfranzke
Copy link
Member

mfranzke commented Jan 6, 2025

@nmerget huger changes like this would need some new chapter within the migration guide (at least as a preparation for an upcoming release).

```

</details>

> **Note:** The `db-ui-42` file contains optional and all components styles. If you consider performance issues see [@db-ui/components](https://www.npmjs.com/package/@db-ui/components) for more information.
> **Note:** The `relative` file contains optional and all components styles. If you consider performance issues see [@db-ui/components](https://www.npmjs.com/package/@db-ui/components) for more information.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
> **Note:** The `relative` file contains optional and all components styles. If you consider performance issues see [@db-ui/components](https://www.npmjs.com/package/@db-ui/components) for more information.
> **Note:** The `@db-ui/components/build/styles/relative` file contains optional and all components styles. If you consider performance issues see [@db-ui/components](https://www.npmjs.com/package/@db-ui/components) for more information.

I think that for clarity it makes sense to include the full path to this instead of just the filename itself.

```

</details>

> **Note:** The `db-ui-42` file contains optional and all components styles. If you consider performance issues see [@db-ui/components](https://www.npmjs.com/package/@db-ui/components) for more information.
> **Note:** The `relative` file contains optional and all components styles. If you consider performance issues see [@db-ui/components](https://www.npmjs.com/package/@db-ui/components) for more information.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
> **Note:** The `relative` file contains optional and all components styles. If you consider performance issues see [@db-ui/components](https://www.npmjs.com/package/@db-ui/components) for more information.
> **Note:** The `@db-ui/components/build/styles/relative` file contains optional and all components styles. If you consider performance issues see [@db-ui/components](https://www.npmjs.com/package/@db-ui/components) for more information.

I think that for clarity it makes sense to include the full path to this instead of just the filename itself.

```

</details>

> **Note:** The `db-ui-42` file contains optional and all components styles. If you consider performance issues see [@db-ui/components](https://www.npmjs.com/package/@db-ui/components) for more information.
> **Note:** The `relative` file contains optional and all components styles. If you consider performance issues see [@db-ui/components](https://www.npmjs.com/package/@db-ui/components) for more information.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
> **Note:** The `relative` file contains optional and all components styles. If you consider performance issues see [@db-ui/components](https://www.npmjs.com/package/@db-ui/components) for more information.
> **Note:** The `@db-ui/components/build/styles/relative` file contains optional and all components styles. If you consider performance issues see [@db-ui/components](https://www.npmjs.com/package/@db-ui/components) for more information.

I think that for clarity it makes sense to include the full path to this instead of just the filename itself.

Copy link
Member

Choose a reason for hiding this comment

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

why has this file been removed ?

preset: [
'default',
{
svgo: false
Copy link
Member

Choose a reason for hiding this comment

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

do you disable this here, because we're already running svgo within the icon repo ?


`db-ui-42` bundles all dependencies from [foundations](https://www.npmjs.com/package/@db-ui/foundations) + all [components](https://github.com/db-ui/mono/blob/main/packages/components/src/styles/db-ui-components.scss) available.
They are bundling all dependencies from [foundations](https://www.npmjs.com/package/@db-ui/foundations) + all [components](https://github.com/db-ui/mono/blob/main/packages/components/src/styles/db-ui-components.scss) available.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
They are bundling all dependencies from [foundations](https://www.npmjs.com/package/@db-ui/foundations) + all [components](https://github.com/db-ui/mono/blob/main/packages/components/src/styles/db-ui-components.scss) available.
They are bundling all dependencies from [foundations](https://www.npmjs.com/package/@db-ui/foundations) and all [components](https://github.com/db-ui/mono/blob/main/packages/components/src/styles/db-ui-components.scss) available.

accessibility (understandability)

Comment on lines +33 to +35
- relative: asset path point to `../assets`
- webpack: asset path point to `~@db-ui/foundations/assets`
- rollup: asset path point to `@db-ui/foundations/assets`
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- relative: asset path point to `../assets`
- webpack: asset path point to `~@db-ui/foundations/assets`
- rollup: asset path point to `@db-ui/foundations/assets`
- `relative`: asset path point to `../assets`
- `webpack`: asset path point to `~@db-ui/foundations/assets`
- `rollup`: asset path point to `@db-ui/foundations/assets`

@@ -0,0 +1,15 @@
/* This is a predefined beginner friendly bundle with default asset-paths (../assets) */
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/* This is a predefined beginner friendly bundle with default asset-paths (../assets) */
/* This is a predefined beginner friendly bundle with default asset-paths (../assets) */

❤️

@@ -0,0 +1,12 @@
@use "internal/custom-elements";

// wc-workaround
Copy link
Member

@mfranzke mfranzke Jan 6, 2025

Choose a reason for hiding this comment

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

Suggested change
// wc-workaround
// Web Components workaround

Not everybody might be familiar that we mean Web Components by the short "wc" phrase. So we should probably do another change for adapting these to the full phrase for clarity both within the comments as well as in the filenames.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏘components Changes inside components folder 📕documentation Improvements or additions to documentation 🏗foundations Changes inside foundations folder 📺showcases Changes to 1-n showcases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants