Pin properties dev tool update #12315

lhsrebel72 · 2023-07-24T19:55:33Z

What does the pull request do?
This has added a pinned variable to property view models and the functionality to use that to pin/unpin properties to the top of dev tools

What is the current behavior?
there is now a checkbox in dev tools that will allow you to pin properties to the top

What is the updated/expected behavior with this PR?
it works as intended other than one thing. It is currently using two headers: one for pinned and another for unpinned properties. I would like to only have one header but, I need to figure out how to bind the widths of each column in the separate data-grids together and have been unable to so far.

How was the solution implemented (if it's not obvious)?
I implemented a check box that is associated with an IsPinned variable on PropertyViewModel. ControlDetailsViewModel will then listen for it and add pinned properties to a separate data-grid above the original one.

Checklist
Added unit tests (if possible)?
Added XML documentation to any related classes?
Consider submitting a PR to https://github.com/AvaloniaUI/Documentation with user documentation
Breaking changes
Hopefully none, fingers crossed

Obsoletions / Deprecations
Fixed issues
Fixes #12242

avaloniaui-team · 2023-07-24T20:18:16Z

You can test this PR using the following package version. 11.0.999-cibuild0038058-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

robloo · 2023-07-24T22:23:01Z

I would put both regulat and attached pinned properties in a new group in the same DataGrid

workgroupengineering

Thanks for your PR, I didn't have time to do it at the time. I don't like the solution of the two headers very much because it increases the complexity. To fix you could just add one more grouping (Pinned/Unpinned) with collapsible.

workgroupengineering · 2023-07-25T07:10:41Z

src/Avalonia.Controls.DataGrid/DataGrid.cs

@@ -4161,7 +4161,7 @@ void SetValidationStatus(ICellEditBinding binding)

            ResetValidationStatus();

-            if (exitEditingMode)
+            if (exitEditingMode && CurrentColumn != null)


I think this change should be included in a separate PR where the reason of change is explained.

lhsrebel72 · 2023-07-25T14:35:46Z

Thanks for your PR, I didn't have time to do it at the time. I don't like the solution of the two headers very much because it increases the complexity. To fix you could just add one more grouping (Pinned/Unpinned) with collapsible.

Do you mean something like <Grid> <DataGrid/> <DataGrid/> <Grid/>
If so, this still shows two headers while both grids are populated since the headers aren't connected you'd never be able to adjust column width for whichever grid had its headers hidden. When the pinned datagrid is unpopulated it is hidden though.

Or more like what is shown here? https://www.c-sharpcorner.com/uploadfile/dpatra/grouping-in-datagrid-in-wpf/

I could also just simply add another groupdescription for IsPinned but, that will not achieve the effect of having the pinned properties always visible while scrolling through non-pinned properties and users would have to scroll back to the top to see them.

And no worries, I enjoy doing it!

workgroupengineering · 2023-07-25T15:46:30Z

Or more like what is shown here? https://www.c-sharpcorner.com/uploadfile/dpatra/grouping-in-datagrid-in-wpf/

I could also just simply add another groupdescription for IsPinned but, that will not achieve the effect of having the pinned properties always visible while scrolling through non-pinned properties and users would have to scroll back to the top to see them.

This

lhsrebel72 · 2023-07-25T15:59:43Z

This

Group description you mean? I can get that pushed shortly!

…estions

lhsrebel72 · 2023-07-26T18:12:53Z

@workgroupengineering @robloo I think i've addressed all concerns with the most recent commit

avaloniaui-team · 2023-07-26T18:55:24Z

You can test this PR using the following package version. 11.0.999-cibuild0038144-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

robloo · 2023-07-27T02:59:06Z

src/Avalonia.Diagnostics/Diagnostics/ViewModels/AvaloniaPropertyViewModel.cs

+            Pinned,
+            Unpinned
+        }
+        private PinnedStatus _isPinned;


Why define this as an enum? The "Is" name prefix implies it is a bool. If you keep it as a bool you can avoid the converter entirely, correct?

robloo · 2023-07-27T03:00:11Z

src/Avalonia.Diagnostics/Diagnostics/ViewModels/AvaloniaPropertyViewModel.cs

+            get => _isPinned.ToString();
+            set
+            {
+                try
+                {
+                    if (Enum.TryParse(value, out PinnedStatus pinState))
+                    {
+                        _isPinned = pinState;
+                    }
+                }
+                catch { }
+            }


It is bad convention to be changing the data type in the property accessors. Whatever the underlying type is really should stay exposed as the property type.

robloo · 2023-07-27T03:00:32Z

src/Avalonia.Diagnostics/Diagnostics/ViewModels/ClrPropertyViewModel.cs

@@ -9,6 +9,12 @@ internal class ClrPropertyViewModel : PropertyViewModel
        private Type _assignedType;
        private object? _value;
        private readonly Type _propertyType;
+        private enum PinnedStatus


I think this enum can go away. Still, you shouldn't have two of the exact same enum defined.

robloo · 2023-07-27T03:01:52Z

src/Avalonia.Diagnostics/Diagnostics/ViewModels/PropertyViewModel.cs

@@ -11,6 +11,7 @@ internal abstract class PropertyViewModel : ViewModelBase
        public abstract string Name { get; }
        public abstract string Group { get; }
        public abstract Type AssignedType { get; }
+        public abstract string IsPinned { get; set; }


So... three type? Bool in ConvertBack, String here and also the Enum?... should be just one as mentioned before. Am I missing something?

So, when it was only a bool passing it in as a groupdescription would have the groups titled true and false rather than pinned and unpinned.
The enum was just an attempt to avoid hardcoding the strings.

If there is another way to have the group descriptions renamed I will implement that but, I wasn't able to find anything. Even just hiding the name of the group would be a good option if possible.

workgroupengineering · 2023-07-27T16:50:41Z

It's a good start, but there are some problems to solve.

The first and most annoying is the loss of the pinned properties when selecting a different control from treeview.
Secondly, the checkboxes take up space reducing the visibility of the columns
Third the property view is not updated until a different property is selected.

The first and most annoying is the loss of the pinned properties when selecting a different control from treeview.

Instead of adding the IsPinned property, you could create a HashSet like this HashSet<(String PropertyName)> pinnedProperties and change your converter to something like this:

    internal class PinnedConverter : IValueConverter
    {
        public HashSet<string> PrinnedProperties { get; set; }

        public object Convert(object? value, Type targetType, object? parameter, CultureInfo culture)
        {
            var isPinned = false;
            if (value is string propertyName)
            {
                isPinned = PrinnedProperties.Contains(propertyName);
            }

            return isPinned ? "Pinned" : "Unpinned";
        }

        public object ConvertBack(object? value, Type targetType, object? parameter, CultureInfo culture)
        {
            return BindingOperations.DoNothing;
        }
    }

and use

private PinnedConverter pinConverter = new();
...

    pinConverter .PrinnedProperties = pinnedProperties

   // view.GroupDescriptions.Add(new DataGridPathGroupDescription(nameof(AvaloniaPropertyViewModel.IsPinned)));
   view.GroupDescriptions.Add(new DataGridPathGroupDescription(nameof(AvaloniaPropertyViewModel.PropertyName))
     {
        Converter = pinConverter ,
     }
   );

Secondly, the checkboxes take up space reducing the visibility of the columns
Third the property view is not updated until a different property is selected.

When a datagrid row is selected, you could add a AdornerDecorator that displays a button that allows you to pin/unpin the property .

lhsrebel72 · 2023-07-28T17:01:02Z

It's a good start, but there are some problems to solve.

The first and most annoying is the loss of the pinned properties when selecting a different control from treeview. Secondly, the checkboxes take up space reducing the visibility of the columns Third the property view is not updated until a different property is selected.

The first and most annoying is the loss of the pinned properties when selecting a different control from treeview.

Instead of adding the IsPinned property, you could create a HashSet like this HashSet<(String PropertyName)> pinnedProperties and change your converter to something like this:
    internal class PinnedConverter : IValueConverter
    {
        public HashSet<string> PrinnedProperties { get; set; }

        public object Convert(object? value, Type targetType, object? parameter, CultureInfo culture)
        {
            var isPinned = false;
            if (value is string propertyName)
            {
                isPinned = PrinnedProperties.Contains(propertyName);
            }

            return isPinned ? "Pinned" : "Unpinned";
        }

        public object ConvertBack(object? value, Type targetType, object? parameter, CultureInfo culture)
        {
            return BindingOperations.DoNothing;
        }
    }
and use
private PinnedConverter pinConverter = new();
...

    pinConverter .PrinnedProperties = pinnedProperties

   // view.GroupDescriptions.Add(new DataGridPathGroupDescription(nameof(AvaloniaPropertyViewModel.IsPinned)));
   view.GroupDescriptions.Add(new DataGridPathGroupDescription(nameof(AvaloniaPropertyViewModel.PropertyName))
     {
        Converter = pinConverter ,
     }
   );
Secondly, the checkboxes take up space reducing the visibility of the columns
Third the property view is not updated until a different property is selected.

When a datagrid row is selected, you could add a AdornerDecorator that displays a button that allows you to pin/unpin the property .

Those seem like great suggestions, I should have some time to get that done and another commit pushed this weekend. Thanks!

lhsrebel72 · 2023-08-08T03:08:27Z

@workgroupengineering I'm getting that wrapped up but, have never worked with adorners before and am having trouble getting that set up. Could you recommend any documentation or a tutorial to help me get that working? Or anywhere where something similar has been done in code would be a great help as well. Thanks!

timunie · 2023-08-08T06:48:03Z

@lhsrebel72 I've seen it in Avalonia.Behaviors for example: https://github.com/AvaloniaUI/Avalonia.Xaml.Behaviors/tree/master/src/Avalonia.Xaml.Interactions.Draggable

workgroupengineering · 2023-08-08T08:00:18Z

@timunie got ahead of me.

lhsrebel72 added 3 commits July 24, 2023 12:30

Added the ability to pin and unpin properties in dev tools

f7e2d24

rebase

1f2c003

minor fixes that slipped through my review

931897b

workgroupengineering reviewed Jul 25, 2023

View reviewed changes

Changed the pinned to be a groupdescription based on code review sugg…

309a132

…estions

Merge branch 'master' into Pin-properties-dev-tool-update

05f3cb9

robloo reviewed Jul 27, 2023

View reviewed changes

workgroupengineering mentioned this pull request Oct 24, 2023

feat(DevTools): Pin properties #13371

Merged

3 tasks

maxkatz6 closed this in #13371 Nov 5, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pin properties dev tool update #12315

Pin properties dev tool update #12315

lhsrebel72 commented Jul 24, 2023

avaloniaui-team commented Jul 24, 2023

robloo commented Jul 24, 2023

workgroupengineering left a comment

workgroupengineering Jul 25, 2023

lhsrebel72 commented Jul 25, 2023 •

edited

Loading

workgroupengineering commented Jul 25, 2023

lhsrebel72 commented Jul 25, 2023

lhsrebel72 commented Jul 26, 2023

avaloniaui-team commented Jul 26, 2023

robloo Jul 27, 2023

robloo Jul 27, 2023

robloo Jul 27, 2023

robloo Jul 27, 2023

lhsrebel72 Jul 27, 2023

workgroupengineering commented Jul 27, 2023

lhsrebel72 commented Jul 28, 2023

lhsrebel72 commented Aug 8, 2023

timunie commented Aug 8, 2023

workgroupengineering commented Aug 8, 2023

Pin properties dev tool update #12315

Pin properties dev tool update #12315

Conversation

lhsrebel72 commented Jul 24, 2023

avaloniaui-team commented Jul 24, 2023

robloo commented Jul 24, 2023

workgroupengineering left a comment

Choose a reason for hiding this comment

workgroupengineering Jul 25, 2023

Choose a reason for hiding this comment

lhsrebel72 commented Jul 25, 2023 • edited Loading

workgroupengineering commented Jul 25, 2023

lhsrebel72 commented Jul 25, 2023

lhsrebel72 commented Jul 26, 2023

avaloniaui-team commented Jul 26, 2023

robloo Jul 27, 2023

Choose a reason for hiding this comment

robloo Jul 27, 2023

Choose a reason for hiding this comment

robloo Jul 27, 2023

Choose a reason for hiding this comment

robloo Jul 27, 2023

Choose a reason for hiding this comment

lhsrebel72 Jul 27, 2023

Choose a reason for hiding this comment

workgroupengineering commented Jul 27, 2023

lhsrebel72 commented Jul 28, 2023

lhsrebel72 commented Aug 8, 2023

timunie commented Aug 8, 2023

workgroupengineering commented Aug 8, 2023

lhsrebel72 commented Jul 25, 2023 •

edited

Loading