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

Move dimension filters to body header; automatically select a process if current one is invalid; design tweaks to accomodate these changes #418

Merged
merged 9 commits into from
Apr 21, 2020

Conversation

hamilton
Copy link
Contributor

@hamilton hamilton commented Apr 18, 2020

Closes #400, closes #401

@hamilton hamilton marked this pull request as ready for review April 20, 2020 18:25
state.productDimensions.process
)
) {
let newProcess = probe.record_in_processes[0];
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it OK to always set it to the first item from the API? Perhaps we should default to parent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure. From inspection, I found that parent is always the first if parent is available to the probe.

@openjck
Copy link
Contributor

openjck commented Apr 20, 2020

This brings our ESLint count down from 113 to 107. 🎉

@@ -243,6 +244,9 @@ h2 {
</div>
{/await}
</div>
<div class='drawer-section drawer-section--end'>
<Button level=low><Doc size={16} /> Documentation</Button>
Copy link
Contributor

Choose a reason for hiding this comment

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

This currently does nothing.

@spasovski
Copy link
Contributor

I think this brings a lot to the table. The new menu buttons have issues (can't immediately click into a new menu, cursor switches to default between clicks) but less issues than the old ones so not a blocker.

I'd just either remove the doc link for now (or comment it out) until it does something...though I am not sure if the documentation is quite ready yet. r+wc

@spasovski spasovski merged commit adc5a43 into master Apr 21, 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
3 participants