search

ABenassi87 / ngx-text-diff

A Text Diff component for Angular
https://ngx-text-diff.herokuapp.com/home
50 stars 44 forks source link

Synchronize scrolling #20 #21

Closed paustint closed 5 years ago

paustint commented 5 years ago

Added the ability for a user to synchronize scrolling of the tables (true be deafault) This requires a new peer dependency of Angular CDK Updated readme with new parameters and improved the example of how to use the library

resolves #20

paustint commented 5 years ago

@ABenassi87 - Check out the changes I have suggested.

This change allows the tables to scroll at the same time instead of individually.

Also, some other suggestions:

  1. You should merge into master so that when people visit the library, they see exactly what is on NPM. Usually it is common to merge to master before pushing to NPM for non-alpha/beta releases
  2. I renamed the checkbox (see in screenshot below)
  3. I added examples to the Readme to show how to use the library, instead of just importing
  4. I think that we might want to revisit how styling works.
    1. Assuming that users of this library have their own style for their application, they will probably want everything to keep the same style.
    2. The buttons and the checkbox use a very specific style, which might not match other libraries.
      1. I would suggest moving some of the CSS to a separate file that the user must either import OR they can modify specific styles (e.x. "theme") if things should be different.

Sorry for my rant above and below, but I think that these items should be considered on the roadmap to ensure the library is not too opinionated. 💯

Scroll Sync

image

Style Example

In my application:

  1. Button/checkbox Colors don't match my application
  2. The checkbox and buttons both have conflicts with my existing stylesheet and don't look the same as the intended look
  3. I might want to show red on both files instead of red and green
  4. I might want to choose the color of red/green
  5. I have a completely different loading style, and I would prefer to never have the loading indicator shown unless I opt-in
  6. I should have the option to get loading events so I can show loading the way my app wants to show loading
  7. I should be able to override the component used for the loading spinner instead of having the look/feel chosen for me image
mercer commented 4 years ago

@ABenassi87

I'd like to use this feature. However, the latest version, 0.5.4, doesn't seem to include it. It's also not documented on https://www.npmjs.com/package/ngx-text-diff

Will you include it in a future release?