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

I made a fork that will be updated #1292

Open
dj-fiorex opened this issue Sep 26, 2021 · 15 comments
Open

I made a fork that will be updated #1292

dj-fiorex opened this issue Sep 26, 2021 · 15 comments

Comments

@dj-fiorex
Copy link

Hi all, i'm an user of this library, so i decided to fork it and I'll take a look at the pull requests and slowly merged everything.
If you want to help me or only support me please take a look at my fork, i will take a look at all prs and add them fast, don't worry :)

@eddyizm
Copy link

eddyizm commented Oct 5, 2021

This is great news!

@tawfiek
Copy link

tawfiek commented Jan 10, 2022

IDK, what's about the backward compatibility in this fork ?!

@eddyizm
Copy link

eddyizm commented Jan 11, 2022

It's a fork moving forward. This one is no longer being updated or worked on. So what happens when this is 5 years stale? @tawfiek Seems like a maintained fork is better than one that is not.

@uap-universe
Copy link

uap-universe commented Feb 28, 2022

IDK, what's about the backward compatibility in this fork ?!

Absolutely devastating. I tried it out, full of hope. But the tables are completely destroyed. The rows are displayed as columns and the layout is all off. Maybe it is a fork worth looking at if you start from scratch, but migration seems impossible.

Update: after investing a lot of work, I achieved results that are somehow okay'ish now. It was definitely not plug and play, but I would not say "impossible" anymore. If you are willing to invest an unpredictable amount of time depending on what you already did with the smart table, you can give it a go.

@dj-fiorex
Copy link
Author

Hello to all, i think that my fork is not "impossible". obviously I prioritized some features that interested me for my use case, but I still think the fork is quite plug and play with the original repo.
I haven't had too much time to include new features in this period but I will be making some updates soon
In the meantime, as always, I'm open to any pull requests that might improve the plugin

@tawfiek
Copy link

tawfiek commented Mar 2, 2022

@tawfiek Seems like a maintained fork is better than one that is not.

Definitely, but I just want to make sure that using this fork won't effect my stable application that is heavily depends on this package.

Me and my team are ready to work on maintaining a stable fork of this one, I think it gonna give us some more flexibility building with smart tables than it does right now.

@dj-fiorex
Copy link
Author

If you agree you and your team could use my fork, so we can work together and make this package even better. I could also add you as owner of the repo if this can make you feel more comfortable

@uap-universe
Copy link

uap-universe commented Mar 21, 2022

A few things to note, that might help:

  1. There is a rowClass attribute which, by the official documentation, defaults to "row". In fact this is a function returning a string (so that the row class can depend on the contents, which is very nice). In the most recent version this function seems to be required (at least in strict mode with proper type checking). But when you add a default function, returning "row", this messes with the style sheet and completely destroys the table layout - so don't do that. Just return empty string instead.
  2. You may need to re-style the action links depending on what styling you used in your project. In particular, you may have to remove the text-decoration: underline if you are using icons for the actions.
  3. It may be necessary to clone the repo and build it on your own for proper integration. We had to do that and change the package name in the package.json from "my-workspace" to "angular2-smart-table". Also, the Angular 13 support had to be patched manually (there is an open PR in @dj-fiorex 's repo that can be applied).

Additionally, in our local fork we added support for SVG content in smart table actions. To enable SVGs, you have to search for the [innerHTML] attribute (where the action button content is inserted) and disable Angular HTML sanitization (e.g. with a special pipe). For some reason I do not know, the Angular sanitizer does not recognize inline SVG as valid HTML.

@dj-fiorex
Copy link
Author

Thanks for your comments, so do you want to help me maintaining this repo?

@uap-universe
Copy link

Yes. It is already planned to provide a PR once we are sure everything is "general enough" (i.e. does not only work in our setting and breaks for others).

@tawfiek
Copy link

tawfiek commented Mar 22, 2022

If you agree you and your team could use my fork, so we can work together and make this package even better. I could also add you as owner of the repo if this can make you feel more comfortable

Definitely, It'll be my pleasure to be a part of this.

@uap-universe
Copy link

Another thing I have noticed (but I have to investigate further):

The editable property does not have any effect. I noticed that the field is called isEditable in IColumn (the same for sort vs. isSortable). If I understand that correctly, every settings object that makes use of sort or editable has to be migrated.

@dj-fiorex
Copy link
Author

Hello guys, i just added @tawfiek and @uap-universe as collaborators in the repo! Hope to start a collaboration that makes this package the best!

@uap-universe yes, we need to refactor that

This was referenced Apr 29, 2022
@cscrum
Copy link

cscrum commented May 11, 2023

Trying to figure out if this will work on Angular 16? I'm having a problem with the Ng2CompleterModule in this and has broken my project after upgrading to Angular16.

@uap-universe
Copy link

@cscrum I am back from my vacations and I have just picked up the work on version 2.9.0 which was supposed to offer Angular 16 support. Unfortunately ngcc has been removed now and various (not only ng2-completer) libraries are still not compatible with Angular Ivy. Therefore, I fear, Angular 16 support will not be available anytime soon.

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

No branches or pull requests

5 participants