Use a proper tool bar in Document Outline#68094
Conversation
ghost
added
Area-IDE
untriaged
|
Approving, predicated reasonable smoke-testing. @ryzngard to take a peek when he has time, to see if there's any questions/concerns on his part. |
|
Can you link to the tracking issue about document outline behavior? |
|
Looking. Did you run https://accessibilityinsights.io/ on the window? |
|
@ryzngard I haven't run the tool. Last time I tried it was a resounding failure (provided no usable information good or bad). |
It's a requirement https://devdiv.visualstudio.com/DevDiv/_wiki/wikis/DevDiv.wiki/11451/Accessibility?anchor=local-testing. If you need help understanding the tool or the results I'm available. |
| _viewModel.SearchText = ""; | ||
| } | ||
|
|
||
| void IVsWindowSearch.ProvideSearchSettings(IVsUIDataSource pSearchSettings) |
There was a problem hiding this comment.
cool, I've never seen this before.
I was able to run the tool and inspect the Document Outline window. Prior to this change, there are 13 failures. After this change, there are 6 failures, with all failures in the tool bar and search box being resolved. |
Great! Thank you for checking and fixing the issues. My hope was embedded would do the right thing, but needed to verify that there was no oddities with the approach |
|
Fixed the remaining items (which were unrelated to this PR but might as well fix them all at once). |
sharwell
added
IDE-Navigation
CyrusNajmabadi
left a comment
CyrusNajmabadi
left a comment
There was a problem hiding this comment.
can you want for me to do a second pass. i would like to see how all the specific issues were addressed.
Thanks for looking at this!
|
@CyrusNajmabadi No problem, I'll keep auto-merge enabled so it will merge as soon as you complete the review. |
This comment was marked as resolved.
This comment was marked as resolved.
| InternalUtilities.SetValue(pSearchSettings, SearchSettingsDataSource.PropertyNames.PrefixFilterMRUItems, false); | ||
| InternalUtilities.SetValue(pSearchSettings, SearchSettingsDataSource.PropertyNames.MaximumMRUItems, (uint)25); | ||
| InternalUtilities.SetValue(pSearchSettings, SearchSettingsDataSource.PropertyNames.SearchWatermark, ServicesVSResources.Document_Outline_Search); | ||
| InternalUtilities.SetValue(pSearchSettings, SearchSettingsDataSource.PropertyNames.SearchPopupAutoDropdown, false); |
There was a problem hiding this comment.
what does this one do?
| { | ||
| // Construct a rectangle at the left of the item to avoid horizontal scrolling when the items is longer than | ||
| // fits in the view. We make the rectangle 25% the width of the containing tree view to ensure at least some | ||
| // of the text is visible for deeply nested items. |
| renderHeight = item.RenderSize.Height; | ||
| } | ||
|
|
||
| var croppedRenderWidth = Math.Min(item.RenderSize.Width, SymbolTree.RenderSize.Width / 4); |
There was a problem hiding this comment.
any concern about this ending up 0? or is that ok?
There was a problem hiding this comment.
I would expect that to be fine. The default value here if you just call BringIntoView() is Rect.Empty.
sharwell
removed
the
untriaged
4 participants
Old behavior:
New behavior after fixing tool bar:
New behavior after fixing tool bar and search box:
Fixes #63859
Closes #67264
Fixes #68096
Fixes #68098
Fixes #68100
Fixes #68102
Fixes #68103
Fixes AB#1680146 (internally-filed accessibility bug related to search box theming)