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

statistics.js: Add global locations and bug fix #2900

Closed
wants to merge 2 commits into from

Conversation

daksh4469
Copy link
Member

@daksh4469 daksh4469 commented Mar 15, 2021

This patch fixes the issue mentioned in the comment of #2896 (#2896 (comment)) and adds global locations of the global variables. This also removes the bullets for cleaner UI. As in every widget, the blocks are not hidden, the same is applied in this.
The commit 35aedab just resolves the conflicts.
Please share your opinions @walterbender @meganindya.
Thanks.

@ksraj123
Copy link
Member

@daksh4469 this was fixed as well in my PR, was a one line change. Working on some stuff. Will add the linting in the last commit of the PR. I suggest that this should be closed to avoid unnecessary merge conflicts. Thanks

@daksh4469
Copy link
Member Author

@daksh4469 this was fixed as well in my PR, was a one line change. Working on some stuff. Will add the linting in the last commit of the PR. I suggest that this should be closed to avoid unnecessary merge conflicts. Thanks

Yeah, it was a one-line change. Sorry, didn't notice this was fixed in your PR too as this was not mentioned in either the issue where I had commented this or in your PR title/description. I would suggest commenting on the issue where this was mentioned that you are working on this so as to avoid these conflicts.
I would close this PR. Add these global locations to your PR.
Also, what do you think of removing bullets and not hiding the blocks when this widget is opened as in the case of every other widget? If you approve these, add this too as a commit in your PR.

Thanks.

@ksraj123
Copy link
Member

Sure, updated the PR comment.

what do you think of removing bullets and not hiding the blocks when this widget is opened

I think the bullets are fine, as sometimes, some fields may wrap beyond a single line, so the bullets give a visual indication of where a field starts and makes the data easily readable. For hiding all blocks when this widget starts, that too would be a one line change but I think there should be a reason behind why it was implemented as such. @walterbender please share your views on this. Thanks

@daksh4469
Copy link
Member Author

Yeah it is a one-line change...but I am with you that it must have been implemented for a reason

@walterbender
Copy link
Member

I think the bullets do help with wrapped text, which is inevitable. Although in full screen, we could probably make the text field wider.

@daksh4469 daksh4469 deleted the daksh4469-main2 branch May 29, 2021 05:05
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.

3 participants