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

Selected Row Visibility #788

Conversation

williamsyang-work
Copy link
Contributor

cropped2

Filter-tree and timeline-chart rows are now highlighted when selected. Selecting a row in the timeline-chart selects the row in the filter-tree, and vice-versa. Highlighted color matches the current color theme.

Requires timeline-chart changes to function: Selected Row Visibility - PR #209

Fixes: #772, #774

Signed-off-by: Will Yang william.yang@ericsson.com

@williamsyang-work
Copy link
Contributor Author

image

build-and-test is failing because the timeline-chart changes I made are not implemented yet.
eclipse-cdt-cloud/timeline-chart#209

@bhufmann
Copy link
Collaborator

@williamsyang-work I'm going to review this and related timeline-chart patch.

@williamsyang-work williamsyang-work force-pushed the sync-timeline-chart-and-tree-selection branch 2 times, most recently from 852c437 to c86ffa3 Compare July 21, 2022 23:08
Copy link
Collaborator

@bhufmann bhufmann left a comment

Choose a reason for hiding this comment

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

It looks good to me. There seems to be build and linting error. Please fix a minor typo, build error and update the yarn.lock to include the new timeline-chart build.

const rowIndex = getIndexOfNode(id, listToTree(this.state.timegraphTree, this.state.columns), this.state.collapsedNodes);
this.chartLayer.selectAndReveal(rowIndex);
if (this.rowController.selectedRow?.id !== id) {
// This hilights the left side if the row is loading.
Copy link
Collaborator

Choose a reason for hiding this comment

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

typo: hilights -> highlights

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved.

@bhufmann
Copy link
Collaborator

It looks good to me. There seems to be build and linting error. Please fix a minor typo, build error and update the yarn.lock to include the new timeline-chart build.

The build error is caused by some network problem. I'll re-trigger build once it's back to normal. The linting error is unrelated and you need to fix it.

@bhufmann
Copy link
Collaborator

It looks good to me. There seems to be build and linting error. Please fix a minor typo, build error and update the yarn.lock to include the new timeline-chart build.

The build error is caused by some network problem. I'll re-trigger build once it's back to normal. The linting error is unrelated and you need to fix it.

The network issue is fixed. There are build and lint errors remaining. @williamsyang-work please update this PR to fix the issues.

@williamsyang-work williamsyang-work force-pushed the sync-timeline-chart-and-tree-selection branch from 783ab28 to 84c6faa Compare July 26, 2022 16:40
Filter-tree and timeline-chart rows are now highlighted when selected.  Selecting a row in the timeline-chart selects the row in the filter-tree, and vice-versa.  Highlighted color matches the current color theme.

Fixes: eclipse-cdt-cloud#772, eclipse-cdt-cloud#774

Signed-off-by: Will Yang <william.yang@ericsson.com>
Signed-off-by: Will Yang <william.yang@ericsson.com>
@williamsyang-work williamsyang-work force-pushed the sync-timeline-chart-and-tree-selection branch from d8b9e27 to 867358e Compare July 26, 2022 18:17
Signed-off-by: Will Yang <william.yang@ericsson.com>
Copy link
Collaborator

@bhufmann bhufmann left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks for the improvement!

@bhufmann bhufmann requested a review from PatrickTasse July 27, 2022 14:38
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.

Improve selection visibility in time graphs
3 participants