-
Notifications
You must be signed in to change notification settings - Fork 226
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
Changes auto-update option added #368
Conversation
c41172d
to
b42aace
Compare
<input | ||
type="checkbox" | ||
id="changes-toggle-polling" | ||
checked={this.state.pollingEnabled ? 'checked="checked"': null} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think this can be: checked={this.state.pollingEnabled}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ooh nice, thanks!
+1 |
f1ecef7
to
b50d027
Compare
getLatestChanges: function () { | ||
var params = {}; | ||
|
||
var lastSeqNum = changesStore.getLastSeqNum(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of default value for getLastSeqNum of null rather make it return now
. Changes supports the now
parameter. Then you don't need to do the if statement and instead just set the since
, feed
and timeout
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just tried this, but for the initial request it can't be a longpoll
or I don't ever get the first batch of data returned; 'now' as a default only works for future changes - for the first request I need the last N results in the past. But I refactored the code a bit to only set it to longpoll
once currentRequest
has been set, and removed the else
. I think it's a bit better now. [Will commit in a moment]
@benkeen this is very cool. Could you add a small notification for each new change that is added. Maybe something like it changes colour slightly just so a user notices that a new set of changes have come through. |
@garrensmith that's a good idea, will do. |
4ff778e
to
75e1a7b
Compare
@benkeen this looks great. I'm getting a strange behaviour if I view changes. Then make a few changes to the db and then click on the filter and check auto update. It then does a lot of updating. If I view the changes page, then check auto update and then make a few changes to the db I don't get as many updates. |
Ah, drat. Good catch. What's happening is that the first time you check the auto update feature it'll show all changes that have occurred since the page was originally loaded, rather than from that moment on which is what you'd expect. I'll fix it. |
7930b3e
to
a34579a
Compare
Squashed & merged. |
This PR adds a "Auto-update changes list" option on the Changes page, within the Filters tab. When enabled, it will automatically update the page with whatever changes occur on the database.
This PR adds a "Auto-update changes list" option on the Changes
page, within the Filters tab. When enabled, it will automatically
update the page with whatever changes occur on the database.