search

kolkov / ngx-gallery

A simple responsive native gallery component for Angular 8+.
https://ngx-gallery.kolkov.ru/
MIT License
115 stars 55 forks source link

Redo: fixing animations on master #27

Closed Simbaclaws closed 4 years ago

Simbaclaws commented 4 years ago

I reverted my master branch back to the commit that was working.

I couldn't squash the commits successfully so please accept this commit from master, everything is working in this branch.

Please review the changes if necessary. The conflict might need to be fixed still.

So far everything seems to be working on my master branch, so I'm not sure if this conflict is necessary.

kolkov commented 4 years ago

A push one commit too )) please resolve conflicts.

Simbaclaws commented 4 years ago

Don't merge yet please, apparently this conflict seems to cause issues.

kolkov commented 4 years ago

Ok, I will waiting for...

Simbaclaws commented 4 years ago

I think this solves the issues, could you check to make sure?

Cloning the repository, doing a npm install, npm run build:lib and npm run start

codecov[bot] commented 4 years ago

Codecov Report

Merging #27 into master will decrease coverage by 0.26%. The diff coverage is 5.26%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #27      +/-   ##
==========================================
- Coverage   35.21%   34.95%   -0.27%     
==========================================
  Files          16       16              
  Lines         798      804       +6     
  Branches      182      184       +2     
==========================================
  Hits          281      281              
- Misses        478      484       +6     
  Partials       39       39
Impacted Files Coverage Δ
...b/ngx-gallery-image/ngx-gallery-image.component.ts 20.57% <0.00%> (-0.24%) :arrow_down:
...x-gallery-preview/ngx-gallery-preview.component.ts 26.54% <20.00%> (-0.48%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 565e756...897576f. Read the comment docs.

kolkov commented 4 years ago

Ok. This fixed image component, but video still continue playing, if click play button in preview component and then closed. Fix this please.

Simbaclaws commented 4 years ago

Okay, now it would stop playing when closing the preview window.

kolkov commented 4 years ago

Thanks! Merged!

Simbaclaws commented 4 years ago

@kolkov

I need this gallery for an app that I'm making, when can we expect the master branch to be up to date with the npm repository here: https://www.npmjs.com/package/@kolkov/ngx-gallery ?

The earlier this can be, the better it would be for me :+1: Perhaps you want to go through some testing stage first.

EDIT: I believe the stackblitz and demo also would need to be updated.

kolkov commented 4 years ago

At the moment, I am consulting with my colleagues in a telegram which version should be issued, minor or major, taking into account the changes that we have made.

Simbaclaws commented 4 years ago

Thank you, please let me know when you have a response :)

EDIT: As soon as it is released, I'll also be testing this inside a webview for both iOS and Android. Just to see whether it has good mobile support. I need this plugin for a mobile app that I'm making.

If mobile support is lacking, I'll help to make it mobile friendly.

kolkov commented 4 years ago

Try to use npm link to use library localy.

Simbaclaws commented 4 years ago

I'd rather wait until it is officially in the repository. I'll make a new branch of my app tomorrow to test things locally with npm link.

Thank you for the suggestion

Simbaclaws commented 4 years ago

@kolkov

Do you perhaps know how I can use this github repository in my project using npm? I'm not quite sure how I can get this to work in my project. I'd rather not create a symbolic link as that would only work on local development, I need these changes in production. What can I do to use the newest version of your library?

I tried doing npm link but it tells me this when building my project: 

ERROR in the target entry-point "ngx-gallery" has missing dependencies: @angular/core @angular/common @angular/platform-browser

Even though I have them installed in the project.

I also tried linking from the dist folder, this would work without dependency issues, but it tells me:

ERROR in Symbol NgClass declared in /Users/doelapp/Desktop/experimental/ngx-gallery/node_modules/@angular/common/common.d.ts is not exported from @angular/common (import into /Users/doelapp/Desktop/experimental/ngx-gallery/dist/gallery/fesm2015/kolkov-ngx-gallery.js

EDIT: I fixed the error message by removing the node_modules folder inside the dist/gallery folder.

EDIT2: now I'm getting: 

core.js:6237 ERROR Error: Uncaught (in promise): Error: BrowserModule has already been loaded. If you need access to common directives such as NgIf and NgFor from a lazy loaded module, import CommonModule instead.

I have CommonModule imported and BrowserModule is imported in my app.module.ts I'm guessing that the plugin imports BrowserModule even though it's already imported by my app.

I have a deadline in about a week so I need to get this to work before then. What keeps you from pushing it to the npm repository?

kolkov commented 4 years ago

This changes is not ready for production yet. Now I thinking about some changes that may cause some API change. If I make this changes I release new major version. What problem to use npm link for production?

Simbaclaws commented 4 years ago

@kolkov

The last error that I'm seeing is:

core.js:6237 ERROR Error: Uncaught (in promise): Error: BrowserModule has already been loaded. If you need access to common directives such as NgIf and NgFor from a lazy loaded module, import CommonModule instead.

But I don't know how to solve it.

Basically I import CommonModule, BrowserModule, BrowserAnimationsModule inside of my app.module.ts

after I've build ngx-gallery with: npm run build:lib and went to the dist gallery folder to npm link and then to my project folder to npm link @kolkov/ngx-gallery.

This would create a @kolkov/ngx-gallery folder inside of my node_modules folder, but I'm guessing some of the files inside of the ngx-gallery import either BrowserModule or BrowserAnimationsModule.

According to this stackoverflow answer: https://stackoverflow.com/questions/45975675/lazy-loading-browsermodule-has-already-been-loaded

The modules should only be imported once.

How can I solve this issue?

kolkov commented 4 years ago

Need repo, to reproduce...

Simbaclaws commented 4 years ago

I'll try creating an example project later on, I can't give my current repo because it's private to our organisation.

I'll try creating a different solution by using a lightbox of some kind instead of a full gallery.

Simbaclaws commented 4 years ago

Hey @kolkov,

Sorry for the late response, I have not looked at this in a long time.

Is there any progress on the new release?

What does still need to be done? I haven't reproduced the issue in my project yet.

I would love to see this released, I'm just curious what you think should be done to make a new release, you said it wasn't ready because it needed API changes.

Which are those exactly?

kolkov commented 4 years ago

I think we need to rewrite this project with a slider component. And we need to move only one slider that contains all slides.

Simbaclaws commented 4 years ago

For me I'm not so sure whether I'd be able to contribute my time into making that happen, it would at least not be in the professional sense that the company I work for wants me to do this in their time. It would become a hobby project in that case.

In any case I don't know whether I want to spend so much time in developing something I can easily find an alternative for online. Since I can probably get another angular carousel from any other source, like for example this one: https://mdbootstrap.com/docs/angular/advanced/carousel/

I understand you put a lot of effort and time in this project, and I appreciate it. But I just don't know whether I'll be either using it, or contributing further to it based on the time that I want to spend on finding a working solution.