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

ui: React: Separate dedupe and partial response checkboxes per panel #2902

Merged
merged 5 commits into from
Jul 28, 2020

Conversation

onprem
Copy link
Member

@onprem onprem commented Jul 16, 2020

  • I added CHANGELOG entry for this change.
  • Change is not relevant to the end user.

Changes

Verification

Tested locally by running unit tests and reviewing network requests.

Screenshots

Before

Deduplication and Partial Response checkboxes were global
image

Now

Moved these options to per panel
image

… panel

Signed-off-by: Prem Kumar <prmsrswt@gmail.com>
Signed-off-by: Prem Kumar <prmsrswt@gmail.com>
@bwplotka
Copy link
Member

Can we have quick screenshot for such changes 🤗 ?

@onprem
Copy link
Member Author

onprem commented Jul 22, 2020

Can we have quick screenshot for such changes 🤗 ?

@bwplotka Done 💯

Copy link
Member

@GiedriusS GiedriusS left a comment

Choose a reason for hiding this comment

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

The changes look good! Just that I think that we forgot to update parseOption and toQueryString so that those options would end up in the URL itself. Let's improve them and I think we can merge this

Signed-off-by: Prem Kumar <prmsrswt@gmail.com>
@onprem onprem changed the title ui: React: Seperate dedup and partial response checkboxes per panel ui: React: Seperate dedupe and partial response checkboxes per panel Jul 23, 2020
@onprem onprem changed the title ui: React: Seperate dedupe and partial response checkboxes per panel ui: React: Separate dedupe and partial response checkboxes per panel Jul 23, 2020
Signed-off-by: Prem Kumar <prmsrswt@gmail.com>
Copy link
Member

@GiedriusS GiedriusS left a comment

Choose a reason for hiding this comment

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

Sorry for being a nuisance but could we change max_source_res to be max_source_resolution so that it would match the old UI? I think there would be some value in being able to copy/paste the params between the UIs. Other than that, I think this is good to go. WDYT?

Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

Amazing ❤️

(modulo @GiedriusS comment)

Signed-off-by: Prem Kumar <prmsrswt@gmail.com>
Copy link
Member

@GiedriusS GiedriusS left a comment

Choose a reason for hiding this comment

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

Awesome work!

@GiedriusS GiedriusS merged commit c25e4cd into thanos-io:master Jul 28, 2020
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.

Query: new ui uses the thanos.io website favicon not the old thanos one
3 participants