[Tree Node Renderer]: Filter button visibility fix

[MartynasStrazdas]

Closes iTwin/viewer-components-react#1117

filter_button_visibility.mp4

Still need to check if visibility button also causes the same bug.

[changeset-bot]

🦋 Changeset detected

Latest commit: ff1a102

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@itwin/presentation-hierarchies-react	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[grigasp]

needs a change file

[saskliutas]

Not sure if it's an issue but in previous version you could expand node and then continue navigating tree using keyboard arrows. With current implementation that is not a case.

[MartynasStrazdas]

Not sure if it's an issue but in previous version you could expand node and then continue navigating tree using keyboard arrows. With current implementation that is not a case.

If you mean by clicking the expand with mouse and then using keys, then it seems it would not be posible with this implementation, but using keys to expand the node and continuing to use them is still good.

But at the same time previously clicking (with mouse) expand did not indicate that the expander is in focus so I don't think it is a big deal.

[saskliutas]

Not sure if it's an issue but in previous version you could expand node and then continue navigating tree using keyboard arrows. With current implementation that is not a case.

If you mean by clicking the expand with mouse and then using keys, then it seems it would not be posible with this implementation, but using keys to expand the node and continuing to use them is still good.

Yes I am talking about expanding node using mouse and then continue with arrow keys.

[grigasp]

Not sure if it's an issue but in previous version you could expand node and then continue navigating tree using keyboard arrows. With current implementation that is not a case.

If you mean by clicking the expand with mouse and then using keys, then it seems it would not be posible with this implementation, but using keys to expand the node and continuing to use them is still good.

Yes I am talking about expanding node using mouse and then continue with arrow keys.

I agree - clicking on the expander should probably focus the node the expander is on, or expander itself. That should also take care of the issue (visible in the demo) where filtering button doesn't go away when expanding child nodes.

[MartynasStrazdas]

Not sure if it's an issue but in previous version you could expand node and then continue navigating tree using keyboard arrows. With current implementation that is not a case.

If you mean by clicking the expand with mouse and then using keys, then it seems it would not be posible with this implementation, but using keys to expand the node and continuing to use them is still good.

Yes I am talking about expanding node using mouse and then continue with arrow keys.

I agree - clicking on the expander should probably focus the node the expander is on, or expander itself. That should also take care of the issue (visible in the demo) where filtering button doesn't go away when expanding child nodes.

Originally the click on expander would focus it, which would mean we'd still have the issue of filter button being visible after expander click.

[grigasp]

Not sure if it's an issue but in previous version you could expand node and then continue navigating tree using keyboard arrows. With current implementation that is not a case.

If you mean by clicking the expand with mouse and then using keys, then it seems it would not be posible with this implementation, but using keys to expand the node and continuing to use them is still good.

Yes I am talking about expanding node using mouse and then continue with arrow keys.

I agree - clicking on the expander should probably focus the node the expander is on, or expander itself. That should also take care of the issue (visible in the demo) where filtering button doesn't go away when expanding child nodes.

Originally the click on expander would focus it, which would mean we'd still have the issue of filter button being visible after expander click.

Could we show the filtering button when the focus is within the node rather than directly on the node?

[MartynasStrazdas]

Not sure if it's an issue but in previous version you could expand node and then continue navigating tree using keyboard arrows. With current implementation that is not a case.

If you mean by clicking the expand with mouse and then using keys, then it seems it would not be posible with this implementation, but using keys to expand the node and continuing to use them is still good.

Yes I am talking about expanding node using mouse and then continue with arrow keys.

I agree - clicking on the expander should probably focus the node the expander is on, or expander itself. That should also take care of the issue (visible in the demo) where filtering button doesn't go away when expanding child nodes.

Originally the click on expander would focus it, which would mean we'd still have the issue of filter button being visible after expander click.

Could we show the filtering button when the focus is within the node rather than directly on the node?

That how it originally works, thats why clicking on expander kept the filter even when hovering on other nodes.

[grigasp]

Not sure if it's an issue but in previous version you could expand node and then continue navigating tree using keyboard arrows. With current implementation that is not a case.

If you mean by clicking the expand with mouse and then using keys, then it seems it would not be posible with this implementation, but using keys to expand the node and continuing to use them is still good.

Yes I am talking about expanding node using mouse and then continue with arrow keys.

I agree - clicking on the expander should probably focus the node the expander is on, or expander itself. That should also take care of the issue (visible in the demo) where filtering button doesn't go away when expanding child nodes.

Originally the click on expander would focus it, which would mean we'd still have the issue of filter button being visible after expander click.

Could we show the filtering button when the focus is within the node rather than directly on the node?

That how it originally works, thats why clicking on expander kept the filter even when hovering on other nodes.

LOL, yeah, you're right, we're back to the original issue.

I clicked around the "Source Control" tree in VSCode. The tree has similar stuff to ours: expanders, action buttons, keyboard navigation, etc. And I think it's pretty intuitive to use, so we should try to make ours similar. Here's what I think is needed:

1. Make expander non-focusable at all. When user clicks on it, the whole node should get focused.
2. When a node is focused, it should always have an outline. This is the key to this issue. It solves two problems that our tree currently has:
   • It's not clear why the filtering button is visible, because focused node has no outline.
   • It's not clear what'll happen when user starts keyboard navigation, because it's not clear what's focused.

Can we do that?

[saskliutas]

Not sure if it's an issue but in previous version you could expand node and then continue navigating tree using keyboard arrows. With current implementation that is not a case.

If you mean by clicking the expand with mouse and then using keys, then it seems it would not be posible with this implementation, but using keys to expand the node and continuing to use them is still good.

Yes I am talking about expanding node using mouse and then continue with arrow keys.

I agree - clicking on the expander should probably focus the node the expander is on, or expander itself. That should also take care of the issue (visible in the demo) where filtering button doesn't go away when expanding child nodes.

Originally the click on expander would focus it, which would mean we'd still have the issue of filter button being visible after expander click.

Could we show the filtering button when the focus is within the node rather than directly on the node?

That how it originally works, thats why clicking on expander kept the filter even when hovering on other nodes.

LOL, yeah, you're right, we're back to the original issue.

I clicked around the "Source Control" tree in VSCode. The tree has similar stuff to ours: expanders, action buttons, keyboard navigation, etc. And I think it's pretty intuitive to use, so we should try to make ours similar. Here's what I think is needed:

1. Make expander non-focusable at all. When user clicks on it, the whole node should get focused.

2. When a node is focused, it should always have an outline. This is the key to this issue. It solves two problems that our tree currently has:

   • It's not clear why the filtering button is visible, because focused node has no outline.
   • It's not clear what'll happen when user starts keyboard navigation, because it's not clear what's focused.

Can we do that?

Maybe we should to get in touch with iTwinUI team regarding missing outline when node becomes focused after expander is clicked?

[grigasp]

Maybe we should to get in touch with iTwinUI team regarding missing outline when node becomes focused after expander is clicked?

I think you're better suited to decide whether it's a bug in iTwinUI or something that should be implemented on our end. Contacting them and asking what they think seems reasonable.

[MartynasStrazdas]

Maybe we should to get in touch with iTwinUI team regarding missing outline when node becomes focused after expander is clicked?

I think you're better suited to decide whether it's a bug in iTwinUI or something that should be implemented on our end. Contacting them and asking what they think seems reasonable.

Tried the solution provided by ItwinUI with some tweaks, this solution would still be affected by the double border (like we see with multi select). This would be a css only solution.

navigation.mp4

@grigasp @saskliutas

[grigasp]

Maybe we should to get in touch with iTwinUI team regarding missing outline when node becomes focused after expander is clicked?

I think you're better suited to decide whether it's a bug in iTwinUI or something that should be implemented on our end. Contacting them and asking what they think seems reasonable.

Tried the solution provided by ItwinUI with some tweaks, this solution would still be affected by the double border (like we see with multi select). This would be a css only solution.

@grigasp @saskliutas

LGTM

[saskliutas]

Maybe we should to get in touch with iTwinUI team regarding missing outline when node becomes focused after expander is clicked?

I think you're better suited to decide whether it's a bug in iTwinUI or something that should be implemented on our end. Contacting them and asking what they think seems reasonable.

Tried the solution provided by ItwinUI with some tweaks, this solution would still be affected by the double border (like we see with multi select). This would be a css only solution.

navigation.mp4
@grigasp @saskliutas

Much better.

[grigasp]

Need a change file