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

Summary row scroll #1479

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

Conversation

SirWojtek
Copy link
Contributor

What kind of change does this PR introduce? (check one with "x")

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

What is the current behavior? (You can also link to an open issue here)
#1462

What is the new behavior?
#1462

Does this PR introduce a breaking change? (check one with "x")

  • Yes
  • No

If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...

Other information:

@TobiasSchulte
Copy link

Is there any chance this could be accepted and merged? Making the sumrow fixed would be a very useful feature..

@cecilyshen
Copy link

Can this be merged? It would be really helpful if I can make the summary row fixed on the top

@ertelovka
Copy link

@SirWojtek what about this merge? SummaryRow fixed is something really needed...

@lgtm-com
Copy link

lgtm-com bot commented Jan 17, 2020

This pull request introduces 7 alerts and fixes 1 when merging 34ab71d into 8a352ed - view on LGTM.com

new alerts:

  • 6 for Syntax error
  • 1 for Unused variable, import, function or class

fixed alerts:

  • 1 for Unused variable, import, function or class

@lgtm-com
Copy link

lgtm-com bot commented Jan 17, 2020

This pull request introduces 1 alert when merging 9c1f3bd into 8a352ed - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

@lgtm-com
Copy link

lgtm-com bot commented Jan 18, 2020

This pull request introduces 1 alert when merging f3fe7c7 into 8a352ed - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

@SirWojtek
Copy link
Contributor Author

SirWojtek commented Jan 18, 2020

Hey @marjan-georgiev, I've rebased the fix and re-checked if it's still relevant. Can I ask you to review it? It has been a long time since I worked with ngx-datatable, I could miss something.

@lgtm-com
Copy link

lgtm-com bot commented Jan 18, 2020

This pull request fixes 1 alert when merging 53706cd into 8a352ed - view on LGTM.com

fixed alerts:

  • 1 for Unused variable, import, function or class

@ratiuandreea
Copy link

When will this feature be available? 😄

@SirWojtek
Copy link
Contributor Author

Hey @ratiuandreea, still waiting for code review. @marjan-georgiev would be nice if you could find some time to take a look at this PR :)

@KeyTurns
Copy link

Is there a plan to merge this at all? Having the same issue with summary row not being shown unless you scroll to the bottom..

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

Successfully merging this pull request may close these issues.

6 participants