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

[Reverted][Experimental] Add icon components #483

Merged
merged 8 commits into from
Mar 25, 2024
Merged

Conversation

Windvis
Copy link
Contributor

@Windvis Windvis commented Mar 19, 2024

This adds icon components for each .svg file in the /public/icons folder. We don't register them in the global component namespace, to use them you need to import them in your JS files and pass them through the template context. This a lot more intuitive in the .gjs world, but it still works in loose-mode as well, but it is more verbose.

This PR isn't very useful by itself, but together with #481 it can be used to replace the string based icon setup.

Warning

This setup is still in an experimental state. We want to try it out in a couple of specific places before considering this ready to use by all the Appuniversum users. We might still break the setup in future minor versions until we announce it as stable.

Usage examples

(These examples assume #481 is merged)

Polaris

// some-delete-button.gjs
import { BinIcon } from '@appuniversum/ember-appuniversum/components/icons/bin';
import AuButton from '@appuniversum/ember-appuniversum/components/au-button';

<template>
  <AuButton
    @icon={{BinIcon}}
    @alert={{true}}
  >
    Delete something
  </AuButton>
</template>

Octane

// some-delete-button.js
import { BinIcon } from '@appuniversum/ember-appuniversum/components/icons/bin';
import Component from '@glimmer/component';

export default class SomeDeleteButton extends Component {
  BinIcon = BinIcon;
}
{{! some-delete-button.hbs}}
<AuButton
  @icon={{this.BinIcon}}
  @alert={{true}}
>
  Delete something
</AuButton>

Part of the solution for #482

const component = `// THIS FILE IS GENERATED. ANY CHANGES TO THIS FILE WILL BE LOST.
import type { TOC } from '@ember/component/template-only';

export interface ${componentName}IconSignature {
Copy link
Contributor

Choose a reason for hiding this comment

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

Since the interface is static, this could live separately and be imported into each file, which would simplify the generated file slightly.

Copy link
Contributor Author

@Windvis Windvis Mar 19, 2024

Choose a reason for hiding this comment

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

Yea, but since all the files are generated it feels a bit strange to store a common type file somewhere, from which they would import. I don't really see an added value, other than DRYing up the generated code a tiny bit, but we would edit the generation script if we would want to make changes anyways, so no real benefit there either. I would prefer to keep it like this for simplicity sake.

I can poke at it if you want. Where would you put the type? We don't really have a "shared" type file in the project yet, AFAIK.

Copy link
Contributor

Choose a reason for hiding this comment

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

It was just a similar strange feeling with generating something that is static, but I don't see a strong reason to pick one feeling over the other.

Windvis added 6 commits March 19, 2024 18:05
- set `mergeAmbiguousCharacters` to true since some of the icons contain numbers in the name: circle-step-1 > CircleStep_1Icon vs CircleStep1Icon
- rename some variables
- shorten some code
I need to double check that this runs when we want, but we were already using it for the symbolset, so I guess it does.
@Windvis Windvis added the enhancement Used when the PR adds a new feature or enhancement. label Mar 20, 2024
@Windvis Windvis changed the title WIP generate icon components [Experimental feature] Add icon components Mar 20, 2024
@Windvis Windvis marked this pull request as ready for review March 20, 2024 11:54
@Windvis Windvis changed the title [Experimental feature] Add icon components [Experimental] Add icon components Mar 20, 2024
It seems the prepare script has some issues and we can't always depend on it for donig the right thing. We'll just run the build script when needed.
@Windvis Windvis merged commit 9e062c7 into master Mar 25, 2024
8 checks passed
@Windvis Windvis deleted the icon-components branch March 25, 2024 11:26
@Windvis
Copy link
Contributor Author

Windvis commented Mar 25, 2024

I forgot to squash merge.. my bad..

@Windvis Windvis removed the enhancement Used when the PR adds a new feature or enhancement. label Mar 25, 2024
@Windvis Windvis changed the title [Experimental] Add icon components [Reverted][Experimental] Add icon components Mar 25, 2024
@Windvis
Copy link
Contributor Author

Windvis commented Mar 25, 2024

Reverted this change and reopened as #487

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.

2 participants