search

mortensimon / web

MIT License
3 stars 11 forks source link

Updated the solution for param filtering. No more inline javascript. Not... #1

Closed ghost closed 10 years ago

ghost commented 10 years ago

... checking for individual key presses which may have affected performance and introduced a laggy feeling to adding text into input field.

mortensimon commented 10 years ago

Thanks a million for your commit. I haven't been able to test it myself, but just looking at it I have a question: Will this work only on the Unit-Configuration page? (I ask since I saw your change in a unit-related-js file). There are similar filter functionality on the following pages (if I'm not mistaken):

This is just a "top-of-my-head" comment, but I thought it would be useful to get a quick comment from you on the topic. If this commit really can speed up things, then great!!

kind regards Morten S

ghost commented 10 years ago

Hi Morten,

The pull request I sent was actually a mistake.

I have forked some of your repos to my account https://github.com/Rikard-Andersson and am working on altering the forks for a project. What I did to the Unit-Config page was to alter the filtering from updating at every keypress event since that introduced a laggy feeling. This may very well be a question about server and client hardware capabilities. My intention was therefore not to commit this specific change to the original repo. However, when in my own fork of your repo tried to pull request between branches, the github webclient defaulted to sending a pull request to the original source of the project. This is why you received a pull request. I tried to delete it but maybe that didnt have effect.

I'm sorry for wasting your time and will make sure to more carefully examine my pull requests in the future :). Also when I actually do come with an alteration that's suitable for the original repo I will make sure to give you a better explanation to what I've done so you may more easily inspect my changes.

Best regards, Rikard

2014-09-08 18:51 GMT+02:00 Morten Simonsen notifications@github.com:

Thanks a million for your commit. I haven't been able to test it myself, but just looking at it I have a question: Will this work only on the Unit-Configuration page? (I ask since I saw your change in a unit-related-js file). There are similar filter functionality on the following pages (if I'm not mistaken):

  • unittype configuration
  • profile configuration
  • group configuration
  • job...maybe

This is just a "top-of-my-head" comment, but I thought it would be useful to get a quick comment from you on the topic. If this commit really can speed up things, then great!!

kind regards Morten S

— Reply to this email directly or view it on GitHub https://github.com/freeacs/web/pull/1#issuecomment-54850539.

mortensimon commented 10 years ago

Thanks again for the sum-up and the best of luck with commits in the future:)

kind regards Morten S

2014-09-09 9:33 GMT+02:00 Rikard Andersson notifications@github.com:

Hi Morten,

The pull request I sent was actually a mistake.

I have forked some of your repos to my account https://github.com/Rikard-Andersson and am working on altering the forks for a project. What I did to the Unit-Config page was to alter the filtering from updating at every keypress event since that introduced a laggy feeling. This may very well be a question about server and client hardware capabilities. My intention was therefore not to commit this specific change to the original repo. However, when in my own fork of your repo tried to pull request between branches, the github webclient defaulted to sending a pull request to the original source of the project. This is why you received a pull request. I tried to delete it but maybe that didnt have effect.

I'm sorry for wasting your time and will make sure to more carefully examine my pull requests in the future :). Also when I actually do come with an alteration that's suitable for the original repo I will make sure to give you a better explanation to what I've done so you may more easily inspect my changes.

Best regards, Rikard

2014-09-08 18:51 GMT+02:00 Morten Simonsen notifications@github.com:

Thanks a million for your commit. I haven't been able to test it myself, but just looking at it I have a question: Will this work only on the Unit-Configuration page? (I ask since I saw your change in a unit-related-js file). There are similar filter functionality on the following pages (if I'm not mistaken):

  • unittype configuration
  • profile configuration
  • group configuration
  • job...maybe

This is just a "top-of-my-head" comment, but I thought it would be useful to get a quick comment from you on the topic. If this commit really can speed up things, then great!!

kind regards Morten S

— Reply to this email directly or view it on GitHub https://github.com/freeacs/web/pull/1#issuecomment-54850539.

— Reply to this email directly or view it on GitHub https://github.com/freeacs/web/pull/1#issuecomment-54934437.