Skip to content

feat: add IonAnimation component - #40

Merged
Lexpeartha merged 20 commits into
nuxt-modules:mainfrom
Lexpeartha:main
Aug 30, 2022
Merged

feat: add IonAnimation component#40
Lexpeartha merged 20 commits into
nuxt-modules:mainfrom
Lexpeartha:main

Conversation

@Lexpeartha

Copy link
Copy Markdown
Collaborator

Resolves#34

I'm looking for a feedback, mostly regarding component's API

This PR is still WIP

@vercel

vercelBot commented Aug 21, 2022

Copy link
Copy Markdown

Someone is attempting to deploy a commit to a Personal Account owned by @danielroe on Vercel.

@danielroe first needs to authorize it.

@codecov-commenter

codecov-commenter commented Aug 21, 2022

Copy link
Copy Markdown

Codecov Report

Merging #40 (d051cf2) into main (8917577) will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@ Coverage Diff @@## main #40 +/- ##
==========================================
+ Coverage 92.18% 92.22% +0.03% 
==========================================
Files 7 8 +1 Lines 448 463 +15 Branches 37 38 +1 ==========================================
+ Hits 413 427 +14 - Misses 35 36 +1 
Impacted FilesCoverage Δ
src/module.ts96.55% <100.00%> (-0.31%)⬇️
src/parts/components.ts100.00% <100.00%> (ø)
src/parts/router.ts92.75% <100.00%> (+0.10%)⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@vercel

vercelBot commented Aug 21, 2022

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

NameStatusPreviewUpdated
nuxt-ionic✅ Ready (Inspect)Visit PreviewAug 25, 2022 at 8:46AM (UTC)

Comment threadsrc/runtime/components.ts Outdated
Comment threadsrc/runtime/components/CreateAnimation.vue Outdated
@Lexpeartha

Copy link
Copy Markdown
CollaboratorAuthor

Worth noting than in future with more components it would make more sense to use addComponentsDir for auto-importing multiple utility components

@Lexpeartha

Copy link
Copy Markdown
CollaboratorAuthor

Also what do you say I create 4th tab in playground showcasing all animations from the ionic docs with this component?
Would that be over-kill?

@danielroe

Copy link
Copy Markdown
Collaborator

4th tab sounds great!

@LexpearthaLexpeartha changed the title feat: add CreateAnimation componentfeat: add IonAnimation componentAug 24, 2022
@Lexpeartha

Copy link
Copy Markdown
CollaboratorAuthor

I think core component is ready. Only thing left is maybe more animation examples, and updating the docs

@danielroedanielroe left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I love it ❤️

Comment threadsrc/parts/components.ts Outdated
Comment threadsrc/runtime/components/IonAnimation.vue
@Lexpeartha

Copy link
Copy Markdown
CollaboratorAuthor

Only thing I'm not really sure how to approach is grouped animation. It allows for using one root animation, to play children animations in parallel. However to do this, I think I need a better way than scoped slot to expose animation object to page/component it is being using in. Any ideas?

@Lexpeartha

Copy link
Copy Markdown
CollaboratorAuthor

@danielroe Could you take a look at docs? Is this enough of documentation for such a component with lots of props, or should I be more thorough?

@Lexpeartha

Lexpeartha commented Aug 26, 2022

Copy link
Copy Markdown
CollaboratorAuthor

Only thing I'm not really sure how to approach is grouped animation. It allows for using one root animation, to play children animations in parallel. However to do this, I think I need a better way than scoped slot to expose animation object to page/component it is being using in. Any ideas?

Also about exposing an animation object, I tried to do some provide/inject trickery to automatically group animations if they are direct children of the another animation component, but to no avail.
Only thing that comes to my mind now is to expose it via emit, but that approach seems very intuitive to me

@LexpearthaLexpeartha self-assigned this Aug 26, 2022
@Lexpeartha

Lexpeartha commented Aug 30, 2022

Copy link
Copy Markdown
CollaboratorAuthor

I'm merging the PR. If you think docs need more work lmk, but I think component is straight-forward enough that this will do

@Lexpeartha
Lexpeartha merged commit 24b1869 into nuxt-modules:mainAug 30, 2022
@github-actionsgithub-actionsBot mentioned this pull request Jun 26, 2026
Sign up for freeto 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.

feat: ionic animation component

3 participants

@Lexpeartha@codecov-commenter@danielroe