Add DataGrid styling

[Bert-Proesmans]

As discussed in #29, I'm attempting to style the DataGrid control.

The intention is to style the following parts of the DataGrid control;

• [X] DataGrid column header
• [X] DataGrid row header (works, but maximum header width sticks. be careful widths of your row header template)
• [X] DataGrid cell error template
• [x] ~DataGrid row groups (recursively)~
• [x] DataGrid 'select all rows' button

This is hard, I'm learning WPF while doing so it might take a while until this task completes. Feedback and help are welcomed!

Bert

[benruehl]

Looks good to me so far. Glad you are joining the project :) Just tell me if you need help with some specific parts.

Just one thing: Is there a reason why you changed some line breaks in the existing ListView styles? Or was this accidently caused by an auto-formatter maybe?

[Bert-Proesmans]

Ah, I didn't intend to commit these changes. I added linebreaks to see all text without horizontal scrolling. I'm working on the PR with additional windows open to compare sources (MS docs, Adonis ListView, etc.)

There are certainly some things I didn't understand at first, but I'm getting there. At the moment I'm guessing that there are implicit features between certain types that break without retaining the invariants. Tough to pinpoint exactly.

[Bert-Proesmans]

Is there a way to enable cooperation for this PR? I need some help because some styles apply, but others don't seem to override the default (like DataGridRowHeader).

[benruehl]

You can make PRs editable for project maintainers. https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork

But maybe you have already. I don't know how to find out if I can edit it other than simply trying to make changes.

[Bert-Proesmans]

Apparantly this was already enabled by default? Anyway, it says users with write access to this repo are allowed to push commits to my PR branch, which is master on my fork.

[benruehl]

Hey, just saw your commit and wanted to inform you that I haven't forgotten this PR. I am kind of busy at the moment and the little time I have I spend in the custom window chrome because multiple people asked for this. As soon as that is finished (very soon probably) I will see if I can help you out over here.

[Bert-Proesmans]

Hi, No problem, I'm still figuring things out myself. There are a few issues marked as TODO in the code, but I'll eventually get it right. Having help just speeds up the process a bit :p

[benruehl]

Hi @Bert-Proesmans

finally I have time to really look into this. Sorry again it took me so long. Thank you for what you have done already. Looks quite good I must say.

What I have done now:

• merged the current master into your fork so that you are up to date
• moved the styles back to the default styles directory
• set the color of the raster lines
• styled the 'select all' button
• Cleaned up a little

I am not sure what is missing though. Seems like you were unsure about the validation templates so I will look into that. There is also some kind of edit mode for cells that should be checked I think. The row header looks good to me. What do you think is missing there (it is marked as todo currently).

[Bert-Proesmans]

Hi, Thanks for reviewing the code so far! I'll run the demo later today and answer the open questions.

Bert

[Bert-Proesmans]

Ok, so I looked at the code and it already looks good.

Following are points to improve the styling even further:

1. The grid row border intersection fix (margin on SelectiveScrollingGrid) gives a high chance of misclicks on row selection. If you click on the bottom line of a row you're clicking on the margin instead of the row itself. The expectation is that the row becomes selected, but that doesn't happen. This expectation is strong because the spotlight layer does go into highlight state at that mouse position.
2. Row header, which includes the validation template, is currently fixed to 16 pixels. This is an allocated width regardless of content. The error template must be bigger (at least 25px) but that's a lot of lost space to always allocate. There must be some way to allow for auto-resizing the entire column of all heads if one of the rows has a visible error template. Grid.SharedSizeGroup will probably be the solution here (this requires Grid.IsSharedSizeScope on the parent). I guess that this is the same feature that drives the AutoSizeColumns property on DataGrid.
3. Swapping theme while a cell is locked in edit mode (eg: due to invalid content) invokes a crash on collection views with the same ItemsSource. I'm not sure if this is anything the library can prevent.
4. Group styles; It seems the idiomatic way to work with groups is to always manually define your own group style. Declaring a CollectionViewSource by itself doesn't change the visual layout. I'm not sure how to implement a default template for groups without additional control code (or complex converters). Maybe declaring a style resource that can be used as BasedOn property would be enough?

I tried to fix point 1 but the SelectiveScrollingGrid must always be on top, which puts the grid border on top.

I only recently discovered Grid.SharedSizeGroup and haven't done much with it yet, but I might be able to integrate this.

[Bert-Proesmans]

Horizontal grippers are currently not present (code commented out). I've never used it myself and am not sure it's that valuable to implement?

[Bert-Proesmans]

I re-enabled the row height grippers and added group sizing on the header column. Auto-sizing works but only when growing, I cannot seem to shrink shared grid column widths when the error template is collapsed. This is an optimization which cannot be worked around i'm afraid.

Also, the select all rows button grows with the parent cell. The default template doesn't do that. Do you want to keep that behaviour?

[benruehl]

The grippers are fine in my opinion. And the 'select all rows' button does not grow anymore, I did not notice this behavior at first.

I see what you mean considering the misclicks on row selection. I did not find a way to do it any better by now. Seems like we have to decide if we want to trade a better look for better usability.

[Bert-Proesmans]

Maybe negative margin on mouse-over layer is an option? I'll look into that tomorrow.

[Bert-Proesmans]

Negative margin is an option, but only for top and bottom offset because the left and right border are noticably not shown. Maybe we want to go for fixed/manually tuned thickness instead of border thickness for padding?

There is also a strange issue with the shared size group:

When leaving edit mode on a cell that triggered the error template the shared size starts to misbehave on virtualized rows. I believe the original row of the position of the errored cell is recycled somehow with uncommited width updates. Editing a cell might duplicate the entire row into staging until the cell validates? Disabling RowVirtualization makes it all work again, but that's not desired for large collections.

Styling this datagrid is hard man.. :p

[benruehl]

The negative margin looks good. But I realized that omitting the border completely looks even better in my opinion. You can try it by setting BorderThickness and Padding to 0 for the GridRow. What do you think?

You are right, the padding should not necessarily depend on the border thickness.

The virtualization issue worries me a bit. I will try to reproduce it on my machine.

PS: Yeah, that control is more complex than I thought as well. But you are doing great! I'm glad you are helping.

[Bert-Proesmans]

You're right, without the padding it looks good as well. Edit: I have committed the changes because i feel it's a good solution for at least the short term. Later on we might want to revisit this decision.

So I was experimenting with GroupStyles and a way for Adonis to have some helpers or preset styles, but group styles is an observable collection on datagrid. There doesn't seem a way to fully style the container or header template, so this falls on the shoulders of the dev.

Next, I wanted to check if the primitive controls inside the group style are getting adonis styles applied but there is a crash regarding cells and the grid scroll host. It seems the group style I applied somehow detached the parent-child relationship between the scroll host and grid cells.

[Bert-Proesmans]

Debugging this is a bit above my head. I have a setup with dotpeek pdb generation and i can mostly understand what the specific method is supposed to do. The tough part is understanding why the scrollhost of the parent datagrid changes between default and adonis. Were we not allowed to change dockpanels/stackpanels into grids in our styles?

[benruehl]

I cannot say what exactly causes the crash but I just noticed that it does not occur when you swap the expander header with the expander content. If I replace the expander with a grid, it works as well. So it might be an issue with the expander's control template, actually.

[Bert-Proesmans]

I'd like to say that I've racked up enough experience with WPF to fix these issues, but I cannot. Literally no further progress was made on my side. I'd like to close this PR, with a reduced scope than originally planned. Hopefully someone else can fill in the missing pieces.

[benruehl]

I feel a little demotivated with this one, too.

I found out that it definitely has something to do with the Expander template because it works when I remove the style from the resources. So I tried to break it further down inside the expander template which revealed that the problem originates from the ScrollViewer "ExpandSiteContainer". Removing all property setters of the ScrollViewer does not change anything, neither does removing all template triggers. The only thing that helps seems to be the removal of said ScrollViewer. Unfortunately, I have no idea how to deal with this situation. It's still just a ScrollViewer, nothing special here, and I don't know how it can lead to a crash.

So, the only possible solution that comes to my mind is to create a separate expander style only for this case which does not have the ScrollViewer. But I agree that we might just merge this PR and adjourn this particular issue.

[Bert-Proesmans]

I don't think having a specific expander style for Datagrid children can be enforced. The programmer is free to design the group style into whatever he wants. If that is somehow achievable we could also define a default Adonis group style, but the only working way I can think of is subclassing the component itself.

About this PR, I'm leaving the decision up to you what to do with the code. You're free to merge or not. I'm not gonna put more effort into it.

[benruehl]

You're right, we could only offer such a style and document when to use it.

I think you achieved a lot here still. It is definitely worth merging. Thanks a lot for all your efforts. And sorry that I was not able to solve the remaining issues as well. I hope it serves your own use case enough so you can use it in your project. May this little unsatisfying end not be the end of all cooperation :)

[benruehl]

Just for future reference:

Even after spending multiple hours with the data grid I wasn't able to resolve the remaining virtualization issue completely.

Conditions required for the issue to occur:

• Enable row virtualization
• Have a validation error in at least one data grid row

Expected behavior:

• The validation error icon appears in the row header
• All other row headers are resized accordingly to take the space the error icon requires

Result:

• Some row headers take the space, some do not (this changes while scrolling)

What I did now is to set the visibility of the error icon of each row to hidden as long as at least one row displays the icon. This makes all other row headers to take the space so that they all are aligned again.

Still, when the validation error is gone, the row headers are expected to shrink again. But instead, again, only some row headers shrink and other do not. Because I was not able to fix this, I implemented a workaround here by making the icon visibility stay on hidden even if all validation errors are gone. This way the row headers take unnecessary empty space forever but at least they stay aligned and the layout does not get broken.

Remember that the issue only occurs if row virtualization is enabled.

[benruehl]

closes #29