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

[Design] Show timeline behind expanded track groups #122

Open
wants to merge 5 commits into
base: reuse_timeline
Choose a base branch
from

Conversation

ALevansSamsung
Copy link
Collaborator

@ALevansSamsung ALevansSamsung commented Jul 1, 2024

What it does

Show summary tracks when track groups are expanded rather then dead space

Based on the Argo designs it seems that rather than highlighting expanded track groups in this way, we would just show an empty track content. From my perspective we could instead display the track group's summary track.
It would look something like this:
image

image

Also markers like notes would show through like normal tracks
image

This will fix this issue: https://github.com/android-graphics/sokatoa/issues/519
May also fix: https://github.com/android-graphics/sokatoa/issues/469

@ALevansSamsung ALevansSamsung changed the title Design/show timeline behind expanded track groups [Design] Show timeline behind expanded track groups Jul 3, 2024
@ALevansSamsung ALevansSamsung marked this pull request as ready for review July 10, 2024 16:03
Copy link
Collaborator

@cdamus cdamus left a comment

Choose a reason for hiding this comment

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

Looks as advertised: the grid is always continuous and the summary track (when present) always shows.

But the selection behaviour of groups is wonky. It requires multiple clicks to deselect and even then, sometimes it doesn't deselect all members of the group. Moreover, when initially drawing the selection, if it only overlaps some of the tracks of an expanded group the selection nonetheless takes in all of the other tracks in that group that I didn't draw the mouse over. See the very beginning of this screen grab for that selection rectangle behaviour.

CleanShot 2024-07-10 at 13 39 14

Copy link
Collaborator

@cdamus cdamus left a comment

Choose a reason for hiding this comment

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

It's not clear to me what has changed ... the selection state of a group as controlled at the group level is still wildly inconsistent.

On the matter of the initial selection range, on reflection I think there's no issue there: the rubber-band is clearly previewing that the whole group will be selected when dragging the pointer over a part of it, so I think that's a clear communication to the user of what will happen.

CleanShot 2024-07-11 at 08 40 18

@ALevansSamsung
Copy link
Collaborator Author

ALevansSamsung commented Jul 11, 2024

It's not clear to me what has changed ... the selection state of a group as controlled at the group level is still wildly inconsistent.

My latest change was suppose to make the checkboxes select and deselect children recursively. I'll test again.

@ALevansSamsung
Copy link
Collaborator Author

Ok, it seems that scrolling over a track group and then also its children causes some tracks to be saved as selected multiple times therefore when they are deselected they are only deselecting the one iteration.

@cdamus
Copy link
Collaborator

cdamus commented Jul 11, 2024

Ok, it seems that scrolling over a track group and then also its children causes some tracks to be saved as selected multiple times therefore when they are deselected they are only deselecting the one iteration.

Oh yeah, that makes sense. In fact, my GIF I think captured this? I notice that the tracks in the "Memory Usage" group that had sticky selection were the ones that my mouse pointer actually dragged over. Others that were included in the extended selection rubber-band were not so sticky.

Copy link
Collaborator

@cdamus cdamus left a comment

Choose a reason for hiding this comment

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

Yup, that did it. Thanks!

@@ -158,20 +158,20 @@ export class PanelContainer implements m.ClassComponent<Attrs> {
startY,
endY);
// Get the track ids from the panels.
const tracks = [];
const tracks = new Set<string>();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice! That's so simple 😀

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.

2 participants