-
Notifications
You must be signed in to change notification settings - Fork 20
logo as image #6661
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
base: main
Are you sure you want to change the base?
logo as image #6661
Changes from 22 commits
53c589e
8889f8a
57fe1c0
cb3b25a
388807c
912bdf2
8a94e58
11cacff
5df7608
a48eaab
4b77f91
ad3a981
a9783c2
3734680
2c0a8fa
142b52e
6ccd935
6d78484
81afed5
a8ae185
826dc7b
944159d
94fd157
45d9e2a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,7 +37,7 @@ | |
| "dev:angular": "nodemon --watch src --watch scripts --watch configs --ext tsx,ts,cjs --exec \"npm run compile:angular\"", | ||
| "dev:html": "pnpm run copy-assets && pnpm run build-assets && pnpm run build-style:01_sass && vite --open", | ||
| "dev:react": "nodemon --watch src --watch scripts --watch configs --ext tsx,ts,cjs --exec \"pnpm run compile:react\"", | ||
| "dev:scss": "pnpm run build-style:01_sass -- --watch --source-map", | ||
| "dev:scss": "sass src:build --load-path=node_modules --watch", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't we keep the |
||
| "dev:stencil": "nodemon --watch src --watch scripts --watch configs --ext tsx,ts,cjs --exec \"pnpm run compile:stencil\"", | ||
| "dev:vue": "nodemon --watch src --watch scripts --watch configs --ext tsx,ts,cjs --exec \"pnpm run compile:vue\"", | ||
| "generate:agent": "mitosis build --config=configs/mitosis.agent.config.cjs", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,14 +7,26 @@ | |
| @extend %db-overwrite-font-size-md; | ||
| @include helpers.display(flex); | ||
|
|
||
| gap: variables.$db-spacing-fixed-sm; | ||
| align-items: center; | ||
| font-weight: 700; | ||
|
|
||
| &::before { | ||
| --db-icon-font-size: #{variables.$db-sizing-md}; | ||
| --db-icon-margin-end: 0; | ||
| --db-icon-color: #{colors.$db-brand-origin-default}; | ||
| &:not(:has(img)) { | ||
|
nmerget marked this conversation as resolved.
|
||
| &::before { | ||
| content: ""; | ||
| block-size: #{variables.$db-sizing-sm}; | ||
| aspect-ratio: var(--db-logo-aspect-ratio); | ||
| background-image: var(--db-logo-url); | ||
| background-repeat: no-repeat; | ||
| background-size: auto #{variables.$db-sizing-sm}; | ||
|
Comment on lines
+15
to
+20
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Logo became |
||
| } | ||
| } | ||
|
|
||
| &:not([data-no-text="true"]) { | ||
| gap: variables.$db-spacing-fixed-sm; | ||
| } | ||
|
|
||
| &[data-no-text="true"] { | ||
| font-size: 0; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This might be an a11y issue. Screenreaders might skip texts with font-size 0. "Do not size content to 0 pixels if you want the content to be read by a screen reader." (webaim)
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I even also already came to that thought, it's good that you remind me of this. We should check our codebase in general, as we're already using it at other places, and want to add the label by |
||
| } | ||
|
|
||
| img { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,7 +9,7 @@ import { StorybookBrandArgTypes } from './_brand.arg.types'; | |
|
|
||
| useMetadata({ | ||
| storybookTitle: 'Variants', | ||
| storybookNames: ['(Default) With Logo', 'No Logo', 'Custom Logo'], | ||
| storybookNames: ['(Default) With Logo', 'Logo Variant', 'Custom Logo'], | ||
| storybookArgTypes: StorybookBrandArgTypes | ||
| }); | ||
|
|
||
|
|
@@ -34,8 +34,10 @@ export default function BrandVariants() { | |
| return ( | ||
| <Fragment> | ||
| <DBBrand>(Default) With Logo</DBBrand> | ||
| <DBBrand hideLogo={true}>No Logo</DBBrand> | ||
| <DBBrand hideLogo={true}> | ||
| <DBBrand data-logo="db-systel" noText> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You added data-logo, but there is no CSS selector in brand.scss that overrides the --db-logo-url. Right now, this example will just render the default DB logo, right? I think we need to add the CSS logic and type the prop. |
||
| Logo Variant | ||
|
Comment on lines
36
to
+38
|
||
| </DBBrand> | ||
|
nmerget marked this conversation as resolved.
|
||
| <DBBrand> | ||
| <img | ||
| src={state.getImage()} | ||
| alt="this is a fancy placeholder logo" | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -2,6 +2,7 @@ import { | |||||
| GlobalProps, | ||||||
| GlobalState, | ||||||
| IconProps, | ||||||
| NoTextProps, | ||||||
| ShowIconProps, | ||||||
| TextProps | ||||||
| } from '../../shared/model'; | ||||||
|
|
@@ -15,9 +16,10 @@ export type DBBrandDefaultProps = { | |||||
|
|
||||||
| export type DBBrandProps = DBBrandDefaultProps & | ||||||
| GlobalProps & | ||||||
| TextProps & | ||||||
| IconProps & | ||||||
| ShowIconProps & | ||||||
|
Comment on lines
20
to
21
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
no more necessary |
||||||
| TextProps; | ||||||
| NoTextProps; | ||||||
|
nmerget marked this conversation as resolved.
Outdated
|
||||||
|
|
||||||
| export type DBBrandDefaultState = {}; | ||||||
|
|
||||||
|
|
||||||
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| // If you serve your assets and your css on root level you might need those paths. | ||
| @use "default.assets-paths" as assets-paths with ( | ||
| $icons-path: "/assets/icons/", | ||
| $fonts-path: "/assets/fonts/" | ||
| $fonts-path: "/assets/fonts/", | ||
| $images-path: "/assets/images/" | ||
| ); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| @use "./default.assets-paths" as assets-paths; | ||
|
|
||
| @mixin get-images { | ||
| --db-textarea-resizer-image: url("#{assets-paths.$images-path}resize_handle_corner.svg"); | ||
| --db-textarea-scrollbar-button-decrement: url("#{assets-paths.$images-path}chevron_down.svg"); | ||
| --db-textarea-scrollbar-button-increment: url("#{assets-paths.$images-path}chevron_up.svg"); | ||
| --db-logo-aspect-ratio: 1; | ||
| --db-logo-url: url("#{assets-paths.$images-path}logo.svg"); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,3 @@ | ||
| $icons-path: "../../assets/icons/" !default; | ||
| $fonts-path: "../../assets/fonts/" !default; | ||
| $images-path: "../../assets/images/" !default; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| // If this library is used as an npm dependency, and the source files should be included rather than css, this corrects the paths. | ||
| @use "default.assets-paths" as assets-paths with ( | ||
| $icons-path: "@db-ux/core-foundations/assets/icons/", | ||
| $fonts-path: "@db-ux/core-foundations/assets/fonts/" | ||
| $fonts-path: "@db-ux/core-foundations/assets/fonts/", | ||
| $images-path: "@db-ux/core-foundations/assets/images/" | ||
| ); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| // If this library is used as an npm dependency, and the source files should be included rather than css, this corrects the paths. | ||
| @use "default.assets-paths" as assets-paths with ( | ||
| $icons-path: "~@db-ux/core-foundations/assets/icons/", | ||
| $fonts-path: "~@db-ux/core-foundations/assets/fonts/" | ||
| $fonts-path: "~@db-ux/core-foundations/assets/fonts/", | ||
| $images-path: "~@db-ux/core-foundations/assets/images/" | ||
| ); |
Uh oh!
There was an error while loading. Please reload this page.