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

example-and-external-config #153

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

artreaktor-niks
Copy link
Collaborator

@artreaktor-niks artreaktor-niks commented Apr 20, 2022

Added changes:

  1. "Static-files" folder for Storybook. Inside this folder we could place logo files and so on - and URL inside twig files will be directly, for example:
    <img src="static-files/logo.svg" />

  2. There is an example of how to add CSS from an external library and for linter exclusion for this file.

  3. added commented code - to add font files.

@@ -2,3 +2,6 @@
to: <%= h.src() %>/<%= h.changeCase.lower(name) %>/.stylelintignore
---
/color/*.css
node_modules/**/*.css
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems useless, because stylelint already ignoring node_module's css (at least it should from what i know)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok. I will remove this.

src:
local(""),
url("../../fonts/ExampleFont.woff2") format("woff2"),
url("../../fonts/ExampleFont.woff") format("woff");
Copy link
Collaborator

Choose a reason for hiding this comment

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

no need for woff already

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

@@ -0,0 +1,4 @@
/* uncomment next line to import css file external */
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure we have to keep it in kaizen. It's not obvious at all for what this file created. Instead - you have to fix ugly bugs and unpredictible behaviors rather than provide some hacks which are clear only to you

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

if we need to include the external CSS files from the library without changing, this is the way to include this without linter check and include this from ONE place. Not in different files from node_modules. If we change something in the component structure we don't need to change it somewhere also. Becasue all files and styles are came from component.

/* uncomment next line to import css file external */
/* @import "@themeName/components/component_example_external"; */

/* stylelint-enable */
Copy link
Collaborator

Choose a reason for hiding this comment

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

This comment is for /static-files/logo.svg file - why it exist here? We already have a logo in theme dir

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed

@artreaktor-niks artreaktor-niks force-pushed the example-and-external-config branch 2 times, most recently from 02d79ae to 90f1fcb Compare April 20, 2022 11:13
@artreaktor-niks
Copy link
Collaborator Author

Removed most of solutions.
Leaved just comment for the font.

@artreaktor-niks artreaktor-niks force-pushed the example-and-external-config branch from 90f1fcb to 6eab55b Compare April 20, 2022 11:15
@iberdinsky-skilld
Copy link
Contributor

please update version, and i'll publish

@artreaktor-niks artreaktor-niks force-pushed the example-and-external-config branch from 6eab55b to 8e4cdd9 Compare May 2, 2022 09:00
@artreaktor-niks
Copy link
Collaborator Author

please update version, and i'll publish

Updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants