Table sticky header

[stsrki]

Based on work from #2214 because it was easier for me to create a separate PR than to refactor the existing. The problem with 2214 PR is that it was changing a lot of table styles and so borderless, stripped, etc would not work.

In this PR I have implemented a working example for Boostrap and Bulma. Other providers will be done soon. Styles changing is kept to a minimum.

@David-Moreira I must admit doing this is a lot harder than anticipated. So far this is the only example with CSS that was working for me. I even considered JavaScript but we'll see, maybe it is not needed. When you have some time please test it. I will continue with work in the meantime. Thanks!

[David-Moreira]

Oh, that's fine if I fucked up. :P I'll take a look when I have the time.

Taking a quick look at commit changes:

    .table-fixed-header thead tr:nth-child(2) th {
        top: 50px;

Isn't header cells height configurable? That's why I used js.

[stsrki]

Oh, that's fine if I fucked up. :P I'll take a look when I have the time.

Taking a quick look at commit changes:

    .table-fixed-header thead tr:nth-child(2) th {
        top: 50px;

Isn't header cells height configurable? That's why I used js.

The fixed height is just for testing purposes. I plan to make it dynamic. Probably the same as your approach with JS.

[David-Moreira]

Hello @stsrki,
Hope you've been doing well! :)

This damn feature does look easier then it actually is, goddamn!
Well, just took a look at bootstrap version. The new table demo is helpfull to test everything together!

• The problem I was asking for help is still there.
• Reminder for the js which most likelly will be needed for the height
• StickyHeader + Bordered seems to thicken up the borders, in retrospective I think I also had this problem, it's easier to notice on this new demo where you can switch between Bordered and no Bordered
• On bordered there are missing borders, not sure if intended, top and bottom and also in between header rows:
  
  -Border+Borderless, this one is interesting... Should it have that one border on the outer edge, but not the one's on top and bottom like before?
  
• With Borderless + Hoverable I can hover inbetween both the header rows. Not very noticeable tough, has to do with that damn problem where a pixel seems to be getting stolen somehow...
  
• Maybe add FixedHeaderTableHeight to demo just to be able to test it being changed dynamically.

---

I was thinking... tables don't have the resize feature like in datagrid, and in retrospective since datagrid uses table behind the scenes, maybe the feature could have been implemented in the table and be available in both, eitherway I was thinking if this is gonna mesh well with it or not, in theory it should still work exactly the same.

---

Not sure if the same is worth doing for the DirtyClasses --> ContainerClassBuilder? As it probably needs to be re-executed for some parameters being changed, and probably on some providers, no?

        /// <inheritdoc/>
        protected override void DirtyStyles()
        {
            ContainerStyleBuilder.Dirty();

            base.DirtyStyles();
        }

As for the code itself, I see you've borrowed some of the stuff I had done. But you're applying your codebase patterns better then I was. :) So looks good to me overall.

[stsrki]

I agree it is not perfect still, and with the new demo, it is easier to compare results. But I think even with some minor border issues it should be ready to merge. If we get complaints later on we can come back and fix it. I don't want to lose too much time on this anymore 😠 Just need to implement dynamic row height with JS.

---

Adding a new table-height parameter might be a good idea. I will add it and it will be used for both sticky and non-sticky mode.

---

Making DirtyClasses --> ContainerClassBuilder is not problematic. It is something I do in a lot of places. It's not a problem from the performance side and it makes a lot of things easier.

[David-Moreira]

Oh I wasn't complaining about it affecting performance. I was saying that you forgot to do it for ContainerClassBuilder it is not refreshing on parameters change.

[stsrki]

JS calculation added now! It's working without any issue as far as I can see.

One thing I'm not sure about is, do we need to call InitializeTableFixedHeader on each render? Is it going to make things slow once we add it to DataGrid? We might render it only on some conditions like first render only, or Narrow change. It might be tricky if table cell size dynamically changes depending on its content.

We'll see. For now, it seems OK.

[David-Moreira]

I'm guessing we would have to do something similar to Dirty() so it only recalculates at the right times.

[stsrki]

Yep, but we'll not optimize too prematurely. Just need to make it work for now.

[stsrki]

I was thinking... tables don't have the resize feature like in datagrid, and in retrospective since datagrid uses table behind the scenes, maybe the feature could have been implemented in the table and be available in both, eitherway I was thinking if this is gonna mesh well with it or not, in theory it should still work exactly the same.

I just now understood this message 🤦‍♂️. It might just be a great idea to have it directly on the Table. I don't think it would be hard to move it. Good catch!