Stay inside viewport #159

[hchiam]

For #159, as per one of the suggestions in pr #528

WIP, to look into:

• perf when there's lots of labels
• offset when label has overflow on the bottom/left

I had to refactor to work with <visbug-labels>, which can't directly access their respective parent/container elements since they're direct descendants of the body

[argyleink]

played around with your WIP featureset here for the past couple days, there's some cool thinking in it! i'm curious if css sticky would work somehow or if maybe these labels with arrows only show up as additional labels instead of trying to be both the inline and out of view label?

they got really crowded and stacked too when making a big selection and some other small stuff, but i'm sure you're aware of them in this WIP pr 👍🏻

but i like the goal, which i'm understanding as hint to where the label is. it does feel tho like #159 is getting a little left behind as this new hint feature is absorbing lots of the work effort. all in all, seems like a worthy ux exploration. hope it's been fun!

[hchiam]

not sure what happened, but when i ran npm ci; npm run extension:build; npm run test:ci locally, all tests passed even without the last fix

[hchiam]

Ready for review again! 😃 Just a max of 8 overflow labels (8 cardinal directions), with counts:

(overflow labels disappear if they have count === 0)

[argyleink]

there's a ton of really great work and ux in here! excited to see it evolve 🙂 most certainly valuable to know that elements are selected out of the viewport.

ux requests / bugs:

• feature request global dismiss (clicking off any of the selected elements) should hide corner labels instead of waiting for a scroll with no selection. they felt sticky.
• feature request show these viewport labels even if the current tool isnt showing labels. i think these labels are different, and serve a more global purpose and therefor dont need filtered out to reduce noise.
• bug after activating the outside viewport label system, hovering elements increments the top left marker on each hover
• feature request maybe top left marker should say "body" instead of "1"?
• bug sometimes, like when i search .artboard, it points to elements outside the left viewport? or that there's some in the bottom left? feels like a bug? i'm not sure what it's trying to tell me. 10 down, but 7 to the bottom left?
• feature request clicking on one of these new labels scroll the viewport do the closest one? (sounds out of scope but i did try clicking the label to be taken to one)

I wonder if we can simplify things a bit and only show up, down, left and right? or help me understand the corners and the reasoning you had when building the ux?!

thanks for the update, had a blast testing this out!

[hchiam]

Thanks! If i understand correctly, here are your thoughts + my thoughts, to-do-list-ified so it's easier for me to keep track:

• [x] 1) feature request "dismiss": make clicking anywhere else on the page hide all these indicator labels?
• [x] 2) feature request "always on": make these indicator labels always show up on the page when VisBug is turned on?
  • [x] (not just behind a plugin, and not just behind one of the main features?)
• [x] 3) bug "ignore hover elements": prevent hovering from incrementing the counts (false positives)
  • [x] NAH: ~maybe try to ignore elements created by VisBug?~
  • [x] NAH: ~and also adjust the calculation to count true overflow, not simply things that are off-screen?.~
• [x] 4) NAH: ~bug "ignore body": check whether the top-left indicator of 1 is a false positive on the body. Try fixing 3) first and consider item 6).~ UPDATE: won't fix, because the label on the body is technically outside the body. If we want to do 2), this makes sense.
• [x] 5) bug "ignore more false positives": check whether the bottom and bottom-left counts are also false positives too.
  • [x] NAH: ~"5)b" I also noticed that running the /zindex plugin more than once also increases the counts, which shouldn't happen.~
• [x] 6) feature request "hover show selector of one culprit in that direction": Users would want to know what element is overflowing, but you might not always be able to scroll to it, like if it's outside the page.
  • [x] So instead of clicking a label to scroll to it, maybe show the selector of the first overflowing element in that direction when you hover over the indicator label for that direction (like how the inspector feature shows a selector when you hover over an element, but instead you're hovering over the label).
  • [x] ~And maybe to manage expectations of click-ability, we should use symbols other than arrows. Could research some cross-compatible emojis or a custom SVG that more clearly indicate overflow and avoid looking like buttons (maybe some crossed-out eye? or an arrow that crosses a dashed line?). Or maybe the arrows will be fine when the hover text is implemented.~
• [x] 7) Why 8 vs 4: I went with separate indicators for the "diagonals" because otherwise you'd have to decide whether to assign them to either horizontal or vertical counts. I also felt that an element with both horizontal and vertical overflow is worse than an element with just either horizontal or vertical overflow, so it felt worth pointing it out separately. But if 8 feels too cluttery I'm fine with simplifying it down to just up/down/left/right. Here's something like the mental model I had in mind for the 8 directions:

3 here	5 things overflowing/offscreen here	↗
←		→
↙	29 things overflowing/offscreen here	↘

Technically it should be true overflow, not simply offscreen stuff ( see 3) ).

[hchiam]

Updated the TODO list above. Addressed most things hopefully.

The tl;dr: this PR currently creates overflow labels that track all the <visbug-label>s added to the DOM by VisBug (when you /zindex, search, etc.).

For 2), @argyleink, let me know if it still makes sense to go ahead with making these overflow labels show up all the time. I think the drawback is we'd always be watching all elements to check if they're overflowing (perf), instead of only watching all VisBug labels to check if they're overflowing.

Besides that, the only other thing for me to look into is to fix the count logic, if you happen to re-run the same search or re-run /zindex ("5)b" in the TODO list). overflowLabel.element.js count

[argyleink]

nice! lots of great improvements here 🙂

2) i'm thinking this is nice =) did you run into any performance issues from it, it seems to work great for me. 5b) imo this is out of your control as plugins cause side effects on a page, a user running /zindex multiple times is going to get the side effects repeated. but, it does expose how some of the logic works, like implementation details leaking into the front end. but i dunno, seems like a small issue to me, we can track it but it's low priority imo.

Remaining items:

• [ ] if i just select one element than hover around, a (͎↑1) shows up at the top of the page, as if there's something there to be shown. i think it's the hover label on the body being counted? seems like it shouldnt show up. i didnt experience this on websites i tried it on, only in the visbug.web.app site.
• [ ] take the random color out and make them the purple label color

Cool, this is super helpful for multi-select when there's many elements selected across a big page 💯 cool feature addition! thanks for hackin on it 👍🏻

[hchiam]

thanks Adam! i found this to be an interesting feature to think about

agreed, the count logic fix is low prio.

I've now filtered out the ↑ labels on the body so the offscreen logic ignores them, but you'll probably see another ↑ label show up for main on the test site. You can now hover over the "offscreen" labels to see the text of one of the labels hidden in that direction.

And sorry, it's been a while and i see i misunderstood 2) and thought we're handling overflow. I recall now that this PR is just handling offscreen labels (https://github.com/GoogleChromeLabs/ProjectVisBug/pull/528#discussion_r659899655). The performance LGTM as-is.