Port asset filters and view options to xplat-editor from Wpf#3090
Port asset filters and view options to xplat-editor from Wpf#3090xianbaum wants to merge 3 commits intostride3d:xplat-editorfrom
Conversation
|
@dotnet-policy-service agree |
|
I just realized there were some files I meant to remove that re-appeared after I tried squashing commits. |
2446c09 to
18c9adc
Compare
18c9adc to
6e1900b
Compare
Kryptos-FR
left a comment
There was a problem hiding this comment.
Thanks for the contribution. It works well. I have made a few remarks but otherwise LGTM.
| </Styles.Resources> | ||
|
|
||
| <!-- Style for SearchComboBox --> | ||
| <Style Selector="sd|SearchComboBox"> |
There was a problem hiding this comment.
It might not have been obvious but I try to keep only editor styles here that are used by the property grid. Other styles go to a different file for now, though they might all get merged together at some point.
Can you extract that style to a new file (e.g. named SearchComboBoxStyle.axaml) and reference it in App.axaml from Stride.GameStudio.Avalonia.
| set => SetValue(AlternativeCommandProperty, value); | ||
| } | ||
|
|
||
| /// <summary>Gets or sets the modifier keys that activate the alternative command.</summary> |
There was a problem hiding this comment.
nit: style preference, usually always on three lines or more. Never inlined. It makes it easier to maintain.
Note: there are a couple more occurrences of it in this file.
There was a problem hiding this comment.
Added newlines for the description for this and the other occurrences.
| @@ -0,0 +1,17 @@ | |||
| // Copyright (c) .NET Foundation and Contributors (https://dotnetfoundation.org/ & https://stride3d.net) and Silicon Studio Corp. (https://www.siliconstudio.co.jp) | |||
There was a problem hiding this comment.
For a new class/type that didn't exist before, we don't add "silicon studio" to the copyright header.
There was a problem hiding this comment.
I removed "and Silicon Studio Corp. (https://www.siliconstudio.co.jp)". The other files are based on existing Wpf code, so I didn't update those.
| } | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the collection of tags associated to this asset. |
There was a problem hiding this comment.
"gets or sets" is incorrect here since it only has a getter. I would only keep "The collection of tags associated to this asset".
There was a problem hiding this comment.
Done! I removed "Gets or sets" in other added summaries, as well.
|
|
||
| /// <inheritdoc/> | ||
| public override int GetHashCode() => | ||
| unchecked(((int)Category * 397) ^ StringComparer.OrdinalIgnoreCase.GetHashCode(Filter)); |
There was a problem hiding this comment.
Better to use the newer Api to compute the hash by combining Category and Filter.
HashCode.Combine(Category, Filter)
I appreciate the feedback. I will get on those requested changes. |
| </FluentTheme> | ||
| <!-- FIXME xplat-editor aggregate all custom styles into a single style/theme --> | ||
| <StyleInclude Source="avares://Stride.Core.Presentation.Avalonia/Themes/EditorStyles.axaml"/> | ||
| <StyleInclude Source="avares://Stride.Core.Presentation.Avalonia/Themes/SearchComboBoxStyle.axaml"/> |
There was a problem hiding this comment.
nit: I'm trying to keep them sorted. It should be moved between hyperlink and textlogviewer.
PR Details
Ports the existing Wpf asset filters and view options. Most of the code is identical to the existing Wpf implementation, but some code was changed or refactored due to Avalonia's differences or new language features.
The following view options were not implemented as the required underlying asset explorer is not functionally complete, but will need to be implemented when the asset explorer is ported over.
I have not added any unit tests yet, but I can do it if requested. Also, happy to implement any feedback.
Related Issue
#2746
Types of changes
Checklist