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

[API Proposal]: Make ItemsControl::OnBringItemIntoView protected #10006

Open
Laniusexcubitor opened this issue Oct 28, 2024 · 2 comments
Open
Labels
API suggestion Early API idea and discussion, it is NOT ready for implementation

Comments

@Laniusexcubitor
Copy link

Background and motivation

When inheriting from ItemsControl, there is no direct way to use the existing API, but it can be useful to have the possibility to use it.

API Proposal

protected internal object OnBringItemIntoView(object arg)
{
    //
}

API Usage

I just want to be able use the same code as in ListBox

public void ScrollIntoView(object item)
{
    if (ItemContainerGenerator.Status == GeneratorStatus.ContainersGenerated)
    {
        OnBringItemIntoView(item);
    }
    else
    {
        // The items aren't generated, try at a later time
        Dispatcher.BeginInvoke(DispatcherPriority.Loaded, new DispatcherOperationCallback(OnBringItemIntoView), item);
    }
}

Alternative Designs

Perhaps a better alternative would be to bring the public method ListBox::ScrollIntoView up to ItemsControl. But make it virtual, as e.g. ComboBox needs some special love (see ComboBoxAutomationProvider::ScrollItemIntoView).

Risks

Could break Applications when developers have a derived ItemsControl where they already have a OnBringItemIntoView method with the same signature. Or in case of the alternative a ScrollIntoView method.

@miloush
Copy link
Contributor

miloush commented Oct 28, 2024

TreeView is a good special case to keep in mind. The selected item is read-only (which is also an issue for #8861), because you would have to do a recursive search to find the container. It is also not enough to just scroll to it, you have to expand all the parents to bring it into view, so the way this can currently be done for TreeView is you get ahold of the container first by any means necessary and then you select it/bring it into view.

In other words, the existing API you want to use would not work for all the controls.

The OnBringItemIntoView either calls BringIntoView on the container, which you can do yourself easily if you have it, or VirtualizingPanel.BringIndexIntoView. For the latter, you need access to the panel resp. ItemsHost and a way to figure out the index and this might be not so trivial. (I would support a public way to access ItemsHost.)

There is nothing wrong with offering a functionality that the derived controls do not have to use as long as they know they can't use it. You cannot bump ListBox.ScrollIntoView up the hierarchy because then you would have to implement the functionality for all the derived controls. It might not be breaking in terms of API surface, but suddenly TreeView and other 3rd party controls would have this method that possibly does not work for them.

So the option is to have this available as protected, but you run into the danger that implementations wouldn't know the limitations and just introduce a public wrapper to it, or call it when it doesn't work like above. That might be an acceptable risk which could be further mitigated with documentation, and possibly a method name. It is also not great that the argument is called arg rather than item when publicly you can only pass item to it, also fixable by different method name (different name would even lower the chance of a conflict).

I am not terribly opposed to the idea, but it's not a great situation either. If making ItemsHost accessible would at least enable people to overcome the gap, that would be my preferred choice, as it has other benefits too.

@Laniusexcubitor
Copy link
Author

Yes I agree, pulling the public API up, was not a clever idea.
Making the ItemsHost accessible is a good idea, currently I inherit VirtualizingStackPanel just to allow to set an own ItemsHost property in my ItemsControls.

@harshit7962 harshit7962 added the API suggestion Early API idea and discussion, it is NOT ready for implementation label Oct 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API suggestion Early API idea and discussion, it is NOT ready for implementation
Projects
None yet
Development

No branches or pull requests

3 participants