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

[Feat]: Angular wrapper #627

Merged

Conversation

donaldxdonald
Copy link
Contributor

Hi @davidjerleke ,

Long time no talk! I apologize for the delay, as I've been quite busy lately. I wanted to let you know that I've finished all the Angular wrapper code and documentation. It's all ready for review. The context is here #18 .

  • embla-carousel-angular
  • embla-carousel-playground-angular
  • docs/get-started/angular

There are a few points that need to be clarified:

Bundling

All library wrapper packages have the same rollup.js structure which bundles the code.

Angular has its own webpack-based Angular CLI for bundling, and since the Angular ecosystem is almost exclusively packaged using the Angular CLI, it makes more sense to use it for bundling.

Publishing

As mentioned above, embla-carousel-angular is packaged using the Angular CLI, which outputs the build to the dist folder (or some other non-root path).

All other libraries in the embla-carousel repository are packaged with rollup and exported directly to the root directory of the library, and then the npm publish command is executed in CI/CD, which is not directly usable in embla-carousel-angular.

So I've changed the release command in cd.yml to run yarn release so that I can use a specific release strategy for each library.

Thank you for your patience, and once again, I apologize for the delay. I appreciate your continued support and guidance throughout this process.

@davidjerleke davidjerleke reopened this Dec 4, 2023
@davidjerleke davidjerleke linked an issue Dec 4, 2023 that may be closed by this pull request
@davidjerleke davidjerleke added feature request New feature or request angular Issue is related to Angular labels Dec 4, 2023
@davidjerleke davidjerleke self-assigned this Dec 4, 2023
@davidjerleke davidjerleke mentioned this pull request Dec 4, 2023
3 tasks
@davidjerleke
Copy link
Owner

@donaldxdonald re-opening this because I've updated this PR with all your changes and fixed the corrupted git history here.

@davidjerleke davidjerleke force-pushed the feat-angular-wrapper branch 2 times, most recently from 2023cec to 7259e79 Compare December 26, 2023 20:44
@zip-fa
Copy link

zip-fa commented Jan 2, 2024

Hi. I was looking for angular wrapper for embla and got here. I saw your code and i have multiple worries about it.

  1. You don't init embla outside Angular (ngZone.runOutsideAngular), which can lead to performance issues; also SSR+hydration can break because of setTimeout calls inside mother library our some plugins api
  2. You don't use inject() function, but instead you use ctor injection (which is not a best practice for now)
  3. You don't use ';'. Which kind of codestyle is this?

I will cherry-pick your code soon and give you link to my repo with everything fixed.

@davidjerleke
Copy link
Owner

davidjerleke commented Jan 2, 2024

@zip-fa thanks for chiming in.

Before you jump into making changes, please discuss it first and don’t assume you know everything better just because you say so. If something is better in your opinion you should be able to make a case and explain why you think it’s better. Don’t get me wrong, I don’t say that your input or ideas aren’t valuable. They probably are, and I don’t doubt that you mean well. But they just need to be discussed before put into action that’s all.

I can give you an answer for these two things:

also SSR+hydration can break because of setTimeout calls inside mother library our some plugins api

The code is using the function canUseDom() before initializing. Have you seen this? It prevents the Embla script from running at all on the server.

You don't use ';'. Which kind of codestyle is this?

This project uses prettier which is an automatic code formatter and it’s configured to not use semi colons. Even though prettier has a little bit of configurable options, they are few and it’s meant to put an end to religious discussions about code formatting so this is not up for discussion to change.

I’m going to let @donaldxdonald answer about point 2 because I don’t know much about how Angular works.

Best,
David

@zip-fa
Copy link

zip-fa commented Jan 2, 2024

https://github.com/zip-fa/ng-embla-carousel

here you go.
We can discuss everything through twitter, maybe it would be better?
What did i do:

  1. Do not use NgModule (because everything is standalone in 2023)
  2. Use afterNextRender method (canUseDom() is redundant; afterNextRender WILL NOT be executed inside SSR)
  3. ngZone.runOutsideAngular will ensure ChangeDetector to not trigger because of Embla's apis
  4. Made your DI token optional (because why we need that much code with default factory? just don't provide token and that's all)

I saw unused @output(), will you use it?

@zip-fa
Copy link

zip-fa commented Jan 2, 2024

Take a look here (still dirty, but main idea shown):

@Input({ required: true })
set ngEmblaCarousel(options: EmblaOptionsType) {
  this.options = options;
  this.reInit();
}

@Input({ alias: 'plugins' })
set _plugins(plugins: EmblaPluginType[]) {
  this.plugins = plugins;
  this.reInit();
}

ngOnChanges usage is redundant. Just use setters, they will be executed each time new value comes.
ngEmblaCarousel is required input, so i made it { required: true }, which enforce user to setup embla (if i got an idea correctly; we can discuss it and remove required option if i am wrong)

Also you have this code in your initial version:
emblaApi!: EmblaCarouselType

But emblaApi can be undefined inside ssr environment.

Sorry if i don't understand something, that's because i found embla-carousel two hours ago :)

@zip-fa
Copy link

zip-fa commented Jan 2, 2024

Sorry for spamming, but one more thing.

Angular has new apis from v16:

  • afterNextRender -> executes only inside browser environment; similar to afterViewInit()
  • DestroyRef.onDestroy() -> replacement to ngOnDestroy(). I bind destroy() callback only when i 100% sure embla is initialized (in browser environment)

There is a lot to do with my implementation, but it's much cleaner in terms of logic (codestyle can be changed to match your repo's style).

@davidjerleke
Copy link
Owner

@zip-fa thank you for your input. If you want to demonstrate your ideas with your code and how you would do it, in order to make them more clear and avoid confusion, you can create a CodeSandbox like @donaldxdonald did here before he created this pull request.

@zip-fa
Copy link

zip-fa commented Jan 3, 2024

@davidjerleke
Copy link
Owner

@zip-fa a CodeSandbox can be tested and forked + edited in matter of a second compared to a GitHub repository that needs to be cloned, setup and it’s not as easy and fast to make iterations and share ideas. Until all agree upon the final product/solution it’s much faster to work with.

@zip-fa
Copy link

zip-fa commented Jan 3, 2024

Got it. I’ll polish everything first and share you a minimal setup with codesandbox.
How about switching our conversation from github to twitter? Mine is: twitter.com/buzahuza

@davidjerleke
Copy link
Owner

davidjerleke commented Jan 3, 2024

@zip-fa I would recommend to keep the conversation here either in a new discussion or in the original issue:

And the reason for this is because it’s better to have everything in one place so devs can see what discussions led to the Angular wrapper becoming reality. And GitHub is the place for code and code related conversations (specifically this repo is relevant) whereas Twitter/X is a place for everything you can think of.

I would also encourage you to read the conversation between me and @donaldxdonald in the original issue to understand what my vision with Embla is and how the other existing framework wrappers work (like react and vue):

@zip-fa
Copy link

zip-fa commented Jan 3, 2024

Hi again.

I polished everything & added stackblitz example with everything working:
https://stackblitz-starters-4buajt.stackblitz.io

Github example updated too (library is buildable & demo working):
https://github.com/zip-fa/ng-embla-carousel

What have been done:

afterViewInit & hasDom() methods removed

Insted, i use afterNextRender hook:

constructor() {
  afterNextRender(
    () => this.init(),
    { phase: AfterRenderPhase.Write }
  );
}

It gets called only inside browser environment, so it's safe to manipulate DOM with it. Link to angular docs

I use inject() function instead of ctor DI

This is the new standard from v14 (which is released few years ago):

private readonly elementRef = inject(ElementRef);
private readonly globalOptions = inject(EMBLA_OPTIONS_TOKEN);
private readonly ngZone = inject(NgZone);
private readonly destroyRef = inject(DestroyRef);

I don't use ngOnDestroy

There is more cleaner approach: DestroyRef.onDestroy(), which will get called on template destruction.

I don't use NgModule

because why should i? standalone apis came to angular from v14 (which is deprecated already, and released few years ago)

I use ngZone.runOutside angular, because of performance-first implementation

scrollTo(index: number, jump?: boolean): void {
  this.ngZone.runOutsideAngular(() => this.emblaApi?.scrollTo(index, jump));
}

scrollPrev(jump?: boolean): void {
  this.ngZone.runOutsideAngular(() => this.emblaApi?.scrollPrev(jump));
}

scrollNext(jump?: boolean): void {
  this.ngZone.runOutsideAngular(() => this.emblaApi?.scrollNext(jump));
}

private init(): void {
  if (this.globalOptions) {
    EmblaCarousel.globalOptions = this.globalOptions;
  }

  this.ngZone.runOutsideAngular(() => {
    this.emblaApi = EmblaCarousel(
      this.elementRef.nativeElement,
      this.options,
      this.plugins
    );

    this.listenEvents();
    this.destroyRef.onDestroy(() => this.destroy());
  });
}

private reInit(): void {
  if (!this.emblaApi) {
    return;
  }

  this.ngZone.runOutsideAngular(() => {
    this.emblaApi!.reInit(this.options, this.plugins);
  });
}

Because of how Angular works, we have to call ngZone.runOutsideAngular to not trigger ChangeDetection mechanism.
zone.js monkey-patches every event: setTimeout, setInterval, etc..., and when setInterval gets called, Angular's ChangeDetector triggers and app performance degrades.

outside_ng_zone.mp4
ng_zone.mp4

Setters removed & ngOnChanges returned

... but with simpler logic. Since the main task is to call reInit() method each time plugins or options @input() changes, i refactored ngOnChanges:

@Input({ alias: 'emblaCarousel' })
public options: EmblaOptionsType = {};

@Input({ alias: 'emblaPlugins' })
public plugins: EmblaPluginType[] = [];
  
ngOnChanges(changes: SimpleChanges) {
  const optionsChange = changes['options'] as SimpleChange | undefined;
  const pluginsChange = changes['plugins'] as SimpleChange | undefined;

  if(
    (optionsChange && !optionsChange.firstChange) ||
    (pluginsChange && !pluginsChange.firstChange)
  ) {
    this.reInit();
  }
}

Removed redundant helpers to compare objects, i just look at firstChange - that's all.

I implemented (emblaChange) event

Now, my directive reacts to every Embla event:

'init',
'pointerDown',
'pointerUp',
'slidesChanged',
'slidesInView',
'select',
'settle',
'destroy',
'reInit',
'resize'

By default, scroll is not included here, because it fires too often. It can be added with provideEmblaOptions in app.config.ts:

providers: [
  provideEmblaOptions({
       eventsThrottleTime: 100, // <-- configure throttle to avoid performance bottlenecks
      listenEvents: [
        'init',
        'pointerDown',
        'pointerUp',
        'slidesChanged',
        'slidesInView',
        'select',
        'settle',
        'destroy',
        'reInit',
        'resize',
        'scroll' // <-- manually add scroll here
      ]
  })
]

As you can see, there is eventsThrottleTime also.
I added throttler based on rxjs in my code, to fix scroll event, which fires too frequently:

private listenEvents(): void {
    const { eventsThrottleTime, listenEvents } = this.globalOptions;

    // This code is valid by the api design: `provideEmblaOptions({ eventsThrottleTime: undefined, listenEvents: undefined })`
    if (!eventsThrottleTime || !listenEvents) {
      return;
    }

    const eventsThrottler$ = new Subject<EmblaEventType>();

    eventsThrottler$
      .pipe(
        throttleTime(eventsThrottleTime),
        takeUntilDestroyed(this.destroyRef)
      )
      .subscribe((eventName) => {
        this.ngZone.run(() => this.emblaChange.emit(eventName));
      });

    this.ngZone.runOutsideAngular(() => {
      listenEvents.forEach((eventName) => {
        this.emblaApi!.on(eventName, () => eventsThrottler$.next(eventName));
      });
    });
  }

@davidjerleke
Copy link
Owner

davidjerleke commented Jan 3, 2024

@zip-fa thanks for your efforts.

Removed redundant helpers to compare objects, i just look at firstChange - that's all.

How exactly does this work under the hood? When is it considered a change? When the reference change?

I implemented (emblaChange) event

Now, my directive reacts to every Embla event

I think you have missed the Embla vision. Please, no event and feature wrappers. Just expose the raw Embla API for maximum flexibility. Please read this comment in detail.

@zip-fa
Copy link

zip-fa commented Jan 3, 2024

How exactly does this work under the hood? When is it considered a change? When the reference change?

Angular compares two objects (old and new), and then ngOnChanges triggers

Please, no event and feature wrappers

That's not how angular 3rd-party libs must work. By not wrapping embla's events and scrollTo/scrollPrev/scrollNext, you will break app performance. I attached two videos on how change detection works in Angular. Please take a look at them, you will notice how many times ChangeDetector is triggered. There is no other way for now, because zone.js is still a part of Angular. Angular's team will make it optional soon, but for now, if not wrap these three methods and every event - RIP for performance.

Also, i published my wrapper to npmjs for those who want to try

Just expose the raw Embla API for maximum flexibility

it is exposed:

@ViewChild(EmblaCarouselDirective)
private readonly emblaCarousel!: EmblaCarouselDirective;

someFunc(): void {
this.emblaCarousel.emblaApi // here it goes
}

You can still access every embla api method, but it will lead to performance issues with manual subscription to events:

this.emblaCarousel.emblaApi.on('scroll', () => console.log('i will kill your app'));
this.emblaCarousel.emblaApi.scrollPrev(); // i will kill your app too
this.emblaCarousel.emblaApi.scrollNext(); // me too!

Preferred way to call scrollPrev/scrollNext:

this.emblaCarousel.scrollPrev(); // i am performant
this.emblaCarousel.scrollNext(); // i am performant too!

Listen to every event in component.html:

<div class="embla__viewport"
   [emblaCarousel]="{ loop: true }"
   (emblaChange)="onEmblaChanged($event)"
>
  <div class="embla__container">
    @for (slide of slideImages(); track $index) {
      <div class="embla__slide">
        <div class="embla__slide__number"><span>{{ $index + 1 }}</span></div>
        <img class="embla__slide__img" [src]="slide" alt="Your alt text">
      </div>
    }
  </div>
</div>

component.ts:

onEmblaChanged(event: EmblaEventType): void {
console.log('no performance bottlenecks: ', event);
}

@davidjerleke
Copy link
Owner

davidjerleke commented Jan 3, 2024

Angular compares two objects (old and new), and then ngOnChanges triggers

So this is a case where I start to doubt your judgement a bit because you say:

Removed redundant helpers to compare objects, i just look at firstChange - that's all.

How do you know they’re redundant? I don’t know if you read the discussion I encouraged you to do but me and @donaldxdonald already discussed this. The utilities are there to prevent the carousel from re-initializing unless options and plugins actually change. It doesn’t care about reference changes but it compares the options and plugin object contents.

You can still access every embla api method, but it will lead to performance issues with manual subscription to events.

Are we talking about micro optimization and performance issues that no one will almost never notice or real problems? It’s hard for me to believe that Angular doesn’t have any way of exposing an API without destroying the browser? Unless there’s proof of a clear performance hit, readability and vision always have the highest priority.

One problem with building another layer of API wrapping is that it needs to be maintained. Every time an event changes or gets added/removed it needs a code change. In contrast, the React, Vue and Svelte wrappers don’t need any manual labor to update as soon as the Embla API changes which is much better in the long run. Again, me and @donaldxdonald have already had this conversation so if you haven’t, please read the conversation here:

Best,
David

@zip-fa
Copy link

zip-fa commented Jan 3, 2024

That's not a micro-optimization. Your setInterval() code will trigger angular to re-check every template each time it executes (every 10ms in your case, check my videos please).
Yes, i've read your conversation, but as i said, your implementation will kill an app without ngZone.runOutsideAngular().

About ngOnChanges: i don't really understand why you want to put this double-check. Angular will not trigger ngOnChanges unless @input() is really changed. Please add console.log() inside ngOnChanges on stackblitz and try to play with its values.

Embla APIs are still exposed, because they are public in @directive() and can be accessed by developers.

I gave you two videos with Angular extension installed, where you can clearly see how it's ChangeDetector triggers, you ask me to prove what? Please do some research about this topic: performance bottlenecks and runOutsideAngular to better understand how Angular's change detection mechanism works. Sorry if it looks a bit rude, but i don't really understand what i have to prove if you didn't even watch my videos.

Best, zip-fa.

@zip-fa
Copy link

zip-fa commented Jan 14, 2024

Yeah, raw api is still exposed. You can play with it how you want. I made full documentation with examples on HOW IT WILL LOOK LIKE in official wrapper here:

https://github.com/zip-fa/ng-embla-carousel/tree/dev (check last paragraph - Access embla api)

Source code is here

What do we do with event listeners? Do you accept my solution?

If we agree on event listeners mechanism and on scrollTo, scrollPrev, scrollNext wrapper, then @donaldxdonald can safely cherry-pick this from my code:

  • my scrollTo, scrollPrev, scrollNext wrappers
  • my listenEvents logic:
@Input()
public subscribeToEvents: EmblaEventType[] = [];

@Input()
public eventsThrottleTime = 100;

@Output()
public readonly emblaChange = new EventEmitter<EmblaEventType>();

  private init(): void {
  // logic here
  this.listenEvents():
  }

/**
   * `eventsThrottler$` Subject was made just because `scroll` event fires too often.
   */
  private listenEvents(): void {
    if (0 === this.subscribeToEvents.length) {
      return;
    }

    const eventsThrottler$ = new Subject<EmblaEventType>();

    eventsThrottler$
      .pipe(
        throttleTime(this.eventsThrottleTime),
        takeUntilDestroyed(this.destroyRef)
      )
      .subscribe((eventName) => {
        this.ngZone.run(() => this.emblaChange.emit(eventName));
      });

    this.ngZone.runOutsideAngular(() => {
      this.subscribeToEvents.forEach((eventName) => {
        this.emblaApi!.on(eventName, () => eventsThrottler$.next(eventName));
      });
    });
  }

@davidjerleke
Copy link
Owner

davidjerleke commented Jan 14, 2024

@zip-fa thanks for the details. I’m going to let @donaldxdonald and @JeanMeche answer your questions here because I don’t know Angular. Let me know when you’re satisfied with the state of this PR and I will just skim through repository related stuff.

I’m always here if you have any questions related to the core library.

Thank you all for your time and efforts!

Best,
David

@donaldxdonald
Copy link
Contributor Author

Update

  • Changed directive's selector to emblaCarousel
  • Added scrollTo, scrollPrev, scrollNext methods
  • Added emblaChange event
  • Updated the embla-carousel-playground-angular
  • Updated Accessing the carousel API and Listening the carousel events in docs/get-started/angular.

scrollTo, scrollPrev, scrollNext methods

To reduce the maintenance cost, the parameters directly use the parameters of the embla API.

@Directive({})
export class EmblaCarouselDirective {

  scrollTo(...args: Parameters<EmblaCarouselType['scrollTo']>): void {
    this.ngZone.runOutsideAngular(() => this.emblaApi?.scrollTo(...args))
  }

  scrollPrev(...args: Parameters<EmblaCarouselType['scrollPrev']>): void {
    this.ngZone.runOutsideAngular(() => this.emblaApi?.scrollPrev(...args))
  }

  scrollNext(...args: Parameters<EmblaCarouselType['scrollNext']>): void {
    this.ngZone.runOutsideAngular(() => this.emblaApi?.scrollNext(...args))
  }

}

cc @zip-fa, @JeanMeche

@zip-fa
Copy link

zip-fa commented Jan 14, 2024

image

@zip-fa
Copy link

zip-fa commented Jan 14, 2024

any chances to get inside contributors list?:))

@donaldxdonald
Copy link
Contributor Author

any chances to get inside contributors list?:))

I agree, but it's up to @davidjerleke .

@davidjerleke
Copy link
Owner

@donaldxdonald, @zip-fa, @JeanMeche you will all be added to the special thanks section in all package README’s, because you’ve made this happen 👍🏻.

@donaldxdonald you will also be added to the regular contributors list, because you created this PR and that list is automatically generated with the build process.

I can add all of you as co-authors when I squash all commits here before merging, but don’t know if that will add all of you as contributors here? Not sure if it works that way.

Best,
David

@zip-fa
Copy link

zip-fa commented Jan 14, 2024

Special thanks section is already very pleasant for me. Thanks!

@davidjerleke davidjerleke self-assigned this Jan 14, 2024
@goetzrobin goetzrobin mentioned this pull request Jan 19, 2024
2 tasks
@davidjerleke davidjerleke changed the title Angular wrapper [Feat]: Angular wrapper Jan 22, 2024
@davidjerleke davidjerleke force-pushed the feat-angular-wrapper branch 2 times, most recently from fb3c059 to 276ee85 Compare January 25, 2024 11:37
Copy link
Owner

@davidjerleke davidjerleke left a comment

Choose a reason for hiding this comment

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

Hi guys @donaldxdonald, @zip-fa, and @JeanMeche,

So I finally got some time off to look into this PR. Great work ⭐!

In addition to the comments I left I have one concern: It doesn't seem like the Angular bundler creates support for moduleResolution: nodeNext? Or maybe I did something wrong? Read more about it here:

Because all embla packages are hybrids and does support that. It's automatically added upon the package build process for each package. I'm just thinking that this problem will just get bigger soon as more devs start using it, and if the Angular build tool isn't doing extraordinary stuff it might be worth using the embla package bundler instead?

I look forward to your feedback.

Best,
David

@donaldxdonald
Copy link
Contributor Author

Hi guys @donaldxdonald, @zip-fa, and @JeanMeche,

So I finally got some time off to look into this PR. Great work ⭐!

In addition to the comments I left I have one concern: It doesn't seem like the Angular bundler creates support for moduleResolution: nodeNext? Or maybe I did something wrong? Read more about it here:

Because all embla packages are hybrids and does support that. It's automatically added upon the package build process for each package. I'm just thinking that this problem will just get bigger soon as more devs start using it, and if the Angular build tool isn't doing extraordinary stuff it might be worth using the embla package bundler instead?

I look forward to your feedback.

Best, David

Hi, @davidjerleke ,

As mentioned earlier, Angular has its own build tool called the Angular CLI, which is based on Webpack (and esbuild starting from Angular v17+). With the Angular CLI, Angular developers don't have to worry about low-level bundling configurations, and the output is compatible with projects using the same or higher Angular versions. For example, embla-carousel-angular is built using Angular CLI v14, which means it can be used in almost all Angular 14+ projects without any issues.

Since its early days, the Angular CLI has been generating bundles only in the ECMAScript Modules (ESM) format (more info), and the Angular team does not recommend using CommonJS format libraries in Angular projects. Reference

It is recommended that you avoid depending on CommonJS modules in your Angular applications. Depending on CommonJS modules can prevent bundlers and minifiers from optimizing your application, which results in larger bundle sizes. Instead, it is recommended that you use ECMAScript modules in your entire application.

Compared to other UI frameworks, Angular is more like a true "framework," and around 99% of Angular projects/libraries use the Angular CLI for bundling. Unlike other frameworks, there are not many variations in "best practices." Therefore, as an Angular library, using the Angular CLI to build embla-carousel-angular is the most suitable approach.

@davidjerleke
Copy link
Owner

@donaldxdonald thanks ⭐! Sorry for the late response. Me and my family got the flu.

About this wrapper - I've been having this nagging feeling that I don't understand how Angular works and therefore I don't understand you guys when you explain things to me. So in order to understand you guys better, I took this Angular course.

I just want to say that I'm sorry if I limited your plans for this Angular wrapper. I know understand that Angular is completely different from React, Vue, Svelte and Solid - Libraries of which I've been part of creating Embla wrappers for. This also made me realise that it makes sense that the Angular wrapper for Embla can be designed very differently from the other wrappers - Whether it should completelty wrap the whole Embla API, its methods and events or not, that's up to you and I won't interfere anything from now on.

I'm working on shared build tools and automating a lot of stuff in this repository for all the other wrappers - Which is doable because they're all very similar in how they work in contrast to Angular. So because Angular is so different in its nature, it might make sense that you @donaldxdonald, create a separate repository for the embla-carousel-angular wrapper? I see many benefits with this approach:

  • No need to always add special conditions for Angular as the Embla build tools and other automated tasks evolve.
  • You can build the Angular wrapper any way you want. Wrap the API entirely or not - I won't interfere at all.
  • You can make changes without waiting for my PR review and approval all the time which means faster changes. This will give you 100% control.
  • You will still have my blessing and even if the Angular wrapper is in a separate repository, it will be the official Angular wrapper for Embla.
  • You are very welcome to add Angular documentation to the official documentation website.
  • All of you guys that have contributed to the Angular wrapper will still be added to the special thanks section in the project README.

What do you think?

Best,
David

@donaldxdonald
Copy link
Contributor Author

Hi @davidjerleke ,

I'm sorry for being busy lately and not replying promptly. As you suggested, creating a separate repository is definitely doable. I'll find some time in the next few days to organize the code. Take care : )

@donaldxdonald
Copy link
Contributor Author

Hi @davidjerleke ,
The initial version of embla-carousel-angular has been released, and I will continue to maintain it. For the documentation I think it would be good to maintain it in one place, in the embla-carousel-angular repository. Thanks for the suggestion.

Repo: https://github.com/donaldxdonald/embla-carousel-angular
NPM: https://www.npmjs.com/package/embla-carousel-angular

@davidjerleke
Copy link
Owner

davidjerleke commented Feb 12, 2024

@donaldxdonald that was fast. Nice work, I think it looks great 🚀! You just got your first ⭐.

For the documentation I think it would be good to maintain it in one place, in the embla-carousel-angular repository.

Makes sense. Let me know if you change your mind.

Is it ok for you if I add the Angular logo and link it to your repository here?

angular

I think it would be good for devs to know that there's an official Angular wrapper out there 🙂.

Best,
David

@donaldxdonald
Copy link
Contributor Author

Is it ok for you if I add the Angular logo and link it to your repository here?

I think it's a great idea. Thanks @davidjerleke !

@davidjerleke
Copy link
Owner

Thanks for all your efforts here @donaldxdonald, @zip-fa and @JeanMeche!

Co-authored-by: David Jerleke <david.jerleke@gmail.com>
Co-authored-by: donaldxdonald <mzhlovefama@gmail.com>
@davidjerleke davidjerleke merged commit 47b0545 into davidjerleke:master Feb 20, 2024
@davidjerleke davidjerleke added the resolved This issue is resolved label Feb 20, 2024
@davidjerleke
Copy link
Owner

davidjerleke commented Feb 29, 2024

@donaldxdonald I think this is cool: The first two users of embla-carousel-angular 🎉!

users

I've added the link to the Angular wrapper now:

angular

Side note

If you have a Reddit account you can share the news that embla-carousel-angular is out in an Angular group or similar. That could increase traction. Nice work ✨!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
angular Issue is related to Angular feature request New feature or request resolved This issue is resolved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feat]: Build an Angular wrapper
4 participants