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

Update to latest ember-appuniversum v3.4.1 #1178

Merged
merged 13 commits into from
Mar 29, 2024
Merged

Conversation

piemonkey
Copy link
Contributor

@piemonkey piemonkey commented Mar 27, 2024

Overview

Passing icons as components instead of strings, as this uses inline SVGs rather than making an extra request for an svgiconset which fails on embeddable.

This allows and requires the latest ember-appuniversum, 3.4.1, as without that, the passing of components to @icon arguments will fail. Any usage of icons using a string argument will result in the svgiconset version being used, which will fail on embeddable.

In order to support importing icons in template only components, added support for .gts single file templates and updated the tooling to add better type checking supported by these.

connected issues and PRs:

Part of lblod/frontend-embeddable-notule-editor#243
Uses the work on appuniversum/ember-appuniversum#482

Setup

You might need to do some set-up in your editor if you haven't set it up to support the new template syntax, details in the ember docs for highlighting and the glint docs for type checking.

In order to test with the new icons imports, you'll need to change the appuniversum version in the test app to 3.4.1.

How to test/reproduce

All the icons should work. If you inspect an icon, you should no longer see a <use> element inside the <svg>, you should just see the svg inline.

Challenges/uncertainties

While refactoring it was easier to write macros that relied on ' quotes being used instead of optionally supporting ", so I converted them. This results in more consistency between files anyway.

There's a bug with some of the new icon components, noted here which I have worked around rather than waiting for a new AU release. This is now fixed in 3.4.1 of AU.

Checks PR readiness

  • UI: works on smaller screen sizes
  • UI: feedback for any loading/error states
  • Check cancel/go-back flows
  • Check database state correct when deleting/updating (especially regarding relationships)
  • changelog
  • npm lint
  • no new deprecations

@piemonkey piemonkey changed the title Update to latest ember-appuniversum v3.4.0 Update to latest ember-appuniversum v3.4.1 Mar 27, 2024
@piemonkey piemonkey force-pushed the internal/latest-au-icons branch from f786f93 to 802479c Compare March 28, 2024 12:31
@piemonkey piemonkey force-pushed the internal/latest-au-icons branch 2 times, most recently from d830106 to 165f706 Compare March 28, 2024 15:14
Enables support for older versions of ember-appuniversum on platforms
that do not need inline svg icons
@piemonkey piemonkey force-pushed the internal/latest-au-icons branch from 165f706 to 627dc56 Compare March 28, 2024 15:18
@elpoelma elpoelma self-requested a review March 29, 2024 10:29
Copy link
Contributor

@elpoelma elpoelma left a comment

Choose a reason for hiding this comment

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

Looks good overall, depending on which version I install in the dummy app, the addon correctly switches between inlining the svg's and use.
I was also able to easily setup the glint language server in vscode.

@elpoelma elpoelma self-requested a review March 29, 2024 13:27
piemonkey and others added 3 commits March 29, 2024 14:46
The test app uses 2.17.0 as this allows us to use the new scss import
paths. 2.15.0 is supported if the client app imports the scss from the
old paths. That makes this set of changes non-breaking.
Co-authored-by: Elena Poelman <elena.poelman@redpencil.io>
@piemonkey piemonkey requested a review from elpoelma March 29, 2024 13:55
@piemonkey piemonkey merged commit 105616a into master Mar 29, 2024
5 checks passed
@piemonkey piemonkey deleted the internal/latest-au-icons branch March 29, 2024 14:30
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