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

[#1485] Watch tab info directly #1490

Merged
merged 4 commits into from
Mar 26, 2021

Conversation

gerhean
Copy link
Contributor

@gerhean gerhean commented Mar 20, 2021

Resolves #1485

Regression by #1390.

Currently zoom tab does not update properly if one of the
zoom watchables do not change.

Let's watch the tabZoomInfo stored in vuex instead of 
zoomOwnerWatchable.

This works as we ensure that every change to tabZoomInfo 
stored in vuex replaces tabZoomInfo with a new object. 

Hence watching the object pointer stored in tabZoomInfo 
is sufficient.

Everything here also applies to the authorship tab with
authorshipOwnerWatchable and tabAuthorshipInfo instead.

Changes in behavior:
Clicking on an already highlighted zoom/authorship tab icon on the summary chart will reset the tab even if there is no changes to summary chart.

@gerhean gerhean changed the title [#1485] Error in connection between chart panel and code panel. Remove zoomOwnerWatchable. Mar 20, 2021
@gerhean gerhean requested a review from a team March 20, 2021 17:48
@damithc
Copy link
Collaborator

damithc commented Mar 20, 2021

Lines in the proposed commit message seem too short i.e., premature line wrapping

@gerhean
Copy link
Contributor Author

gerhean commented Mar 20, 2021

I have increased the length thanks.

@HCY123902
Copy link
Contributor

HCY123902 commented Mar 21, 2021

Not sure about the original specified behavior. However, unticking a checkbox in the break down file type currently does not seem to synchronize the corresponding file type in the zoom panel without reloading the page.

@gerhean
Copy link
Contributor Author

gerhean commented Mar 22, 2021

@HCY123902 I believe that the zoom tab has never actually been responsive to changes in summary-charts until the zoom tab has been reloaded. Hence your suggestions seems more like a new feature? Similarly, changing the date range in summary does not affect the zoom tab.

Furthermore the authorship tab does not have this feature either.

@gerhean gerhean changed the title Remove zoomOwnerWatchable. Remove tab ownerWatchable. Mar 22, 2021
@gerhean gerhean changed the title Remove tab ownerWatchable. Watch tab info directly. Mar 22, 2021
@gerhean gerhean changed the title Watch tab info directly. Watch tab info directly Mar 22, 2021
Copy link
Contributor

@HCY123902 HCY123902 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this pull request can be merged before #1488 because it seems that #1488 is indeed handling a slightly different issue

@fzdy1914 fzdy1914 requested a review from a team March 23, 2021 06:08
Copy link
Contributor

@0blivious 0blivious left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change the PR title according to the DG

@fzdy1914 fzdy1914 changed the title Watch tab info directly [#1485] Watch tab info directly Mar 26, 2021
@fzdy1914 fzdy1914 merged commit 4778111 into reposense:master Mar 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error in connection between chart panel and code panel.
5 participants