-
Notifications
You must be signed in to change notification settings - Fork 86
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(sketch-generator): add icon merging to generator #324
Conversation
a9936cf
to
6f38dbc
Compare
</div> | ||
<div class="agent-states--item"> | ||
<p class="agent-states--label">:focus</p> | ||
<scale-link data-sketch-key="link-12" icon="{{>icon}}" href="#" data-sketch-state="focus">Link Text</scale-link> | ||
<scale-link data-sketch-key="link-12" data-has-icon="true" href="#" data-sketch-state="focus">Link Text <scale-icon-navigation-external-link | ||
size="16" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unfortunately, prettier has no support for handlebars but you could run the html parts through to tidy up the formatting of this file - the indentation is really off here 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
did end up settings the handebars file associaction and applied html formatting
sidenote: we should move on from hbs templates, i have something leaner in mind and will explore replacement soon 🙃
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please test if everything still works after formatting - prettier might do weird things to the hbs code.
thanks for the PR Stefan, it's a lot of effort from you and is really appreciated! it's difficult to tell what is really going on in this diff without a PR description so I left only style-related comments. I see a lot of conditions which are difficult to follow - maybe some inline code comments would help to reason about nested |
c3cbc8b
to
3190e68
Compare
@@ -0,0 +1,8 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we want this file checked in to git?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could be
depends if we want to stay IDE agnostic^^
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
..and again, since handlebar templates will hopefully replaced, this can go
</scale-card> | ||
<script> | ||
/* I couldn't find a way to do this with "theming" */ | ||
document.querySelector('[data-no-padding]').styles = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there a way of getting rid of styles
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, we need to take a look at the requirements for this
this is also related to #344
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this a blocker then?
@@ -62,3 +62,143 @@ See [Conventional Commits](https://conventionalcommits.org) for commit guideline | |||
# [3.0.0-beta.2](https://github.com/telekom/scale/compare/v0.1.4...v3.0.0-beta.2) (2021-04-20) | |||
|
|||
**Note:** Version bump only for package @telekom/scale-generator-sketch | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you remove these additions which are before v3, please?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
absolutely, wasn't sure if we wanted to keep them^^
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but yeah, nothing meaningful in there 😅
Co-authored-by: Ilmari Heikkinen <[email protected]>
also set prettier als default fortmatter for vscode
3190e68
to
d1d95f5
Compare
Co-authored-by: Ilmari Heikkinen [email protected]