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.

Sign up for GitHub

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

Jump to bottom

fix: Chart loaders on reload and change of time interval at dashboard #2068

Merged
merged 35 commits into from
Jan 25, 2023
Merged

fix: Chart loaders on reload and change of time interval at dashboard #2068

merged 35 commits into from
Jan 25, 2023

Conversation

techchintan
Copy link
Contributor

@techchintan techchintan commented Jan 18, 2023 •
edited by palashgdev
Loading

Close #2073
Close #2074

@request-info
Copy link

request-info bot commented Jan 18, 2023

We would appreciate it if you could provide us with more info about this issue/pr!

@techchintan techchintan self-assigned this Jan 18, 2023
@techchintan
Copy link
Contributor Author

@palashgdev @ankitnayan Link of fix issue video
https://www.loom.com/share/96ec59a56c624e61b1bab0d70cca125e

@techchintan techchintan changed the title fix: Chart data logic on dashboard reloads fix: Chart loaders on reload and change of time interval at dashboard Jan 19, 2023
@ankitnayan ankitnayan requested review from srikanthccv and removed request for ankitnayan January 19, 2023 04:35
srikanthccv
srikanthccv previously approved these changes Jan 19, 2023
View reviewed changes
Copy link
Member

@srikanthccv srikanthccv left a comment

Choose a reason for hiding this comment

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

LGTM

@ankitnayan
Copy link
Collaborator

@pranay01 with this design, how would a user know if the new api call failed as we will continue showing old data at the panels?

@ankitnayan
Copy link
Collaborator

https://www.loom.com/share/d67d6ea759994b0eb28d7f9d9d1c74d9

@ankitnayan
Copy link
Collaborator

@csudani7 I also don't see the step type changing on choosing 1 day or 1 week interval

@techchintan
Copy link
Contributor Author

@ankitnayan @palashgdev Updated link : covered error state:
https://www.loom.com/share/6c00ac585a1a4c6d95a0d78f8c594bd5

Please re-check again

@ankitnayan
Copy link
Collaborator

@csudani7 are all 3 issues fixed?

palashgdev
palashgdev previously approved these changes Jan 25, 2023
View reviewed changes
@palashgdev palashgdev requested review from makeavish, jyash97 and palashgdev and removed request for jyash97 January 25, 2023 09:02
@makeavish
Copy link
Member

@csudani7 Can you please check this behaviour?
https://www.loom.com/share/c990f425c1a049539d2a473b5ec98b41

@palashgdev
Copy link
Contributor

https://www.loom.com/share/c990f425c1a049539d2a473b5ec98b41

No @makeavish i don't think we have impact over this area.. in this one...we can make a separate issue to look into this issue separately

@sonarcloud
Copy link

sonarcloud bot commented Jan 25, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
15.4% 15.4% Duplication

@palashgdev palashgdev merged commit 213838a into SigNoz:develop Jan 25, 2023
@palashgdev palashgdev mentioned this pull request Jan 25, 2023
Closed
@techchintan techchintan deleted the fix/issue-2004 branch February 3, 2023 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jyash97 jyash97 jyash97 left review comments

@palashgdev palashgdev palashgdev approved these changes

@srikanthccv srikanthccv Awaiting requested review from srikanthccv srikanthccv is a code owner

@makeavish makeavish Awaiting requested review from makeavish

Assignees

@techchintan techchintan

Labels
ok-to-test request-more-info testing-deploy
Projects
None yet
Milestone
No milestone
6 participants
@techchintan @ankitnayan @palashgdev @makeavish @jyash97 @srikanthccv