Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
1843 new ajax request resets whole view if historypanel is enabled #1872
1843 new ajax request resets whole view if historypanel is enabled #1872
Changes from 8 commits
096fdf4
f499e41
73c96eb
def324a
150e11b
0419cd7
c6d3b65
bd2a24e
d23bbd2
5c3da4c
1d2ed26
4cb7900
20fad64
59e6a28
43a6966
ba03e9f
0fa7647
ea556df
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
It's not completely consistent but we generally prefer using
camelCase
in JavaScript code.Also, other parts of the code use the dataset attribute, so this should be
djDebug.dataset.updateOnAjax
anddata-update-on-ajax
(or whatever the name of the attribute is). Adata-
prefix for attributes is a good idea anyway because such attributes are always valid HTML.I wonder if there is a better place for this property though. Tim suggested using localStorage to allow switching between the auto update and manual update behavior on the fly, and this probably wouldn't work with the way it's proposed here. Also, I don't think we want to expose the attribute to external modification (just yet). Putting it in the same place as the
handleDragged
property may be a good idea?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.
For some context, when Elineda and I spoke yesterday, we decided we could start with a settings control. If we find that it doesn't meet developers needs, we can add the JS to make it more stateful.
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.
Makes sense!
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 did change asked, but I don't put on a localstorage, I just with handleDragged.