RFD: Ellipsization support in wxGrid

[Vadim Zeitlin]

Hello,

I'd like to implement support for displaying ellipsis in the cells that
are too narrow to show their full contents in wxGrid instead of either
overflowing or just truncating it, as it happens now, i.e. make it behave
as wxDataViewCtrl does.

The question is what would be the most suitable API for specifying whether
the contents of the given grid cell should be ellipsized (and, if so, how:
beginning, middle or end?) or not and I see at least the following
possibilities, listed with their advantages and drawbacks:

1. Just copy wxDVC API and add ellipsization mode to wxGridCellRenderer.
+ Consistent with wxDataViewRenderer::EnableEllipsize().
- Non-orthogonal with wxGridCellAttr overflow mode, i.e. combination of
overflowing and ellipsizing doesn't make sense.

2. Create a separate wxGridCellEllipsizeStringRenderer
+ Consistent with the existing wxGridCellAutoWrapStringRenderer.
- Only applies to the text, but not numbers.

3. Add ellipsization mode to wxGridCellAttr.
+ Solves the conflict between overflowing/ellipsizing mentioned above.
- It doesn't seem very logical to make it part of the attribute.

For me the crucial question is whether we consider having the "overflow"
flag in wxGridCellAttr a good idea or not. If it is, then we probably ought
to put "ellipsize" flag there too, for the reasons of consistency and
orthogonality. But if we think it was a mistake to have "overflow" there in
the first place, we probably shouldn't compound it.

Personally I dislike having "overflow" in wxGridCellAttr, but not strongly
enough to be sure that it's really wrong. And for my personal use it does
make size to have "ellipsize" flag there because I want to just globally
turn it on for all cells by setting it in the default attribute. So I'm
probably going to choose the solution (3), but please let me know if you
see anything other wrong with it or, perhaps, see some solution (4) that
would be even better.

TIA!
VZ

[Iwbnwif Yiw]

On Tuesday, January 7, 2020 at 5:42:02 PM UTC, VZ wrote:

 

1. Just copy wxDVC API and add ellipsization mode to wxGridCellRenderer.
 + Consistent with wxDataViewRenderer::EnableEllipsize().
 - Non-orthogonal with wxGridCellAttr overflow mode, i.e. combination of
   overflowing and ellipsizing doesn't make sense.

2. Create a separate wxGridCellEllipsizeStringRenderer
 + Consistent with the existing wxGridCellAutoWrapStringRenderer.
 - Only applies to the text, but not numbers.

3. Add ellipsization mode to wxGridCellAttr.
 + Solves the conflict between overflowing/ellipsizing mentioned above.
 - It doesn't seem very logical to make it part of the attribute.

To start with a caveat, I am barely using wxGrid these days in favor of wxDVC - so don't really have a very strong view.

That said, option 2) sounds like the best one to me. wxGrid behaves quite differently to wxDVC so I don't see it really necessary to be consistent for this one function (i.e. option 1) + isn't a strong benefit). 

Option 2 seems to also offer 100% backwards capability with any existing code. It also matches well the concept of any enhanced cell display is provided by a dedicated renderer. It could be argued that ellipsization is 'core' rather than 'extended' behavior, but wxGrid has survived this long - so it probably isn't that core!

Another advantage to option 2 is the ability to extend the API - for example to configure left, right, mid-string ellipsis and other things in future.

[Vadim Zeitlin]

On Thu, 9 Jan 2020 03:25:28 -0800 (PST) Iwbnwif Yiw wrote:

IY> On Tuesday, January 7, 2020 at 5:42:02 PM UTC, VZ wrote:
IY>
IY> > 1. Just copy wxDVC API and add ellipsization mode to wxGridCellRenderer.
IY> > + Consistent with wxDataViewRenderer::EnableEllipsize().
IY> > - Non-orthogonal with wxGridCellAttr overflow mode, i.e. combination of
IY> > overflowing and ellipsizing doesn't make sense.
IY> >
IY> > 2. Create a separate wxGridCellEllipsizeStringRenderer
IY> > + Consistent with the existing wxGridCellAutoWrapStringRenderer.
IY> > - Only applies to the text, but not numbers.
IY> >
IY> > 3. Add ellipsization mode to wxGridCellAttr.
IY> > + Solves the conflict between overflowing/ellipsizing mentioned above.
IY> > - It doesn't seem very logical to make it part of the attribute.
IY> >
IY>
IY> To start with a caveat, I am barely using wxGrid these days in favor of
IY> wxDVC - so don't really have a very strong view.

Thanks nevertheless for your thoughts!

IY> That said, option 2) sounds like the best one to me.

The trouble is that I want to be able to ellipsize numbers and dates too
and I don't see how to make this work with this approach. Having separate
renderers for these types just doesn't seem like a good idea, but how else
to do it?

IY> Option 2 seems to also offer 100% backwards capability with any existing
IY> code.

AFAICS all options are backwards compatible, at least I don't see how
could they affect the existing code.

IY> It also matches well the concept of any enhanced cell display is
IY> provided by a dedicated renderer. It could be argued that ellipsization is
IY> 'core' rather than 'extended' behavior, but wxGrid has survived this long -
IY> so it probably isn't that core!
IY>
IY> Another advantage to option 2 is the ability to extend the API - for
IY> example to configure left, right, mid-string ellipsis and other things in
IY> future.

This can be done with all the options. I.e. EnableEllipsize() would take
the mode in (1) and we'd add wxGridCellAttr::SetEllipsizeMode() with (3).
So this is really not specific to (2).

Regards,
VZ

[Iwbnwif Yiw]

On Thursday, January 9, 2020 at 3:19:24 PM UTC, VZ wrote:

IY> That said, option 2) sounds like the best one to me.

 The trouble is that I want to be able to ellipsize numbers and dates too
and I don't see how to make this work with this approach. Having separate
renderers for these types just doesn't seem like a good idea, but how else
to do it?

Hmm, I see the problem. 

I suppose you could convert those types to a string in your application, or subclass wxGridCellEllipsizeStringRenderer in your application. It depends if ellipsized numbers and dates, without any string content, is likely to be a common requirement.

 

IY> Option 2 seems to also offer 100% backwards capability with any existing
IY> code.

 AFAICS all options are backwards compatible, at least I don't see how
could they affect the existing code.

Okay, assuming the default setting is for no ellipsization and there is no change to the wxGridCellAttr ctor (for example). My point is that by having a new class it is almost guaranteed that there will be no interference with existing applications, whereas by modifying an existing class there is a slight risk that something is overlooked. I know - very weak ;)

 

IY> It also matches well the concept of any enhanced cell display is
IY> provided by a dedicated renderer. It could be argued that ellipsization is
IY> 'core' rather than 'extended' behavior, but wxGrid has survived this long -
IY> so it probably isn't that core!
IY>
IY> Another advantage to option 2 is the ability to extend the API - for
IY> example to configure left, right, mid-string ellipsis and other things in
IY> future.

This can be done with all the options. I.e. EnableEllipsize() would take
the mode in (1) and we'd add wxGridCellAttr::SetEllipsizeMode() with (3).
So this is really not specific to (2).

I was thinking about have different display options for the ellipses - e.g. "... ..." or "---" or some other format see https://en.wikipedia.org/wiki/Ellipsis#Computer_representations. Of course, these can be added into wxGridCellAttr too if needed, but it might start to get messy.

Anyway, I am playing devil's advocate here - really just to be a 'sounding board' for these ideas. Option 3) is pretty fine - but one more thing - will you implement ellipsizing in all subclasses of wxGridCellRenderer, including wxGridCellAutoWrapStringRenderer and wxGridCellEnumRenderer? If not, what will be the behavior of SetEllipsizeMode() for those renderers?

[Vadim Zeitlin]

Hello again,

For the curious, and to (maybe?) finish with this thread, the proposed
changes are at

https://github.com/wxWidgets/wxWidgets/pull/1705

Any comments on this PR are welcome, as usual!
VZ

[Iwbnwif Yiw]

On Saturday, January 11, 2020 at 6:20:40 PM UTC, VZ wrote:

For the curious, and to (maybe?) finish with this thread, the proposed
changes are at

        https://github.com/wxWidgets/wxWidgets/pull/1705

Nice, and definitely feels quite 'wxGridey' :)

I can only think of wxGridCellOverflowMode instead of wxGridFitMode as the function is really about how the grid treats overflowing content. 

If that is too long or you are not happy clashing with previous naming, then I would suggest wxGridCellFitMode because it is the cells where the 'fitting' is taking place, rather than fitting the grid per se (e.g. into a sizer). This is also more consistent with functions such as SetCellFitMode which include 'cell' in the name.

 

[Vadim Zeitlin]

On Sun, 12 Jan 2020 02:49:15 -0800 (PST) Iwbnwif Yiw wrote:

IY> On Saturday, January 11, 2020 at 6:20:40 PM UTC, VZ wrote:
IY>
IY> For the curious, and to (maybe?) finish with this thread, the proposed
IY> > changes are at
IY> >
IY> > https://github.com/wxWidgets/wxWidgets/pull/1705
IY> >
IY>
IY> Nice, and definitely feels quite 'wxGridey' :)

Thanks!

IY> I can only think of wxGridCellOverflowMode instead of wxGridFitMode as the
IY> function is really about how the grid treats overflowing content.

Yes, but...

IY> If that is too long or you are not happy clashing with previous naming,

... I indeed was afraid of confusion between SetCellOverflowMode() and the
existing SetCellOverflow(), so I didn't want to call it like this. Also,
SetCellOverflow(wxGridOverflowMode::Ellipsize()) doesn't seem logical: it
doesn't actually overflow if we ellipsize the string.

Maybe I'm overthinking this, and it would be definitely simpler to use a
single term ("overflow") for everything, but I just can't convince myself
that it's really clearer than using "fit".

IY> then I would suggest wxGridCellFitMode because it is the cells where the
IY> 'fitting' is taking place, rather than fitting the grid per se (e.g. into a
IY> sizer). This is also more consistent with functions such as SetCellFitMode
IY> which include 'cell' in the name.

I've intentionally avoided putting "Cell" in the enum name because in
theory we could use it for the headers (rows/labels) too. I don't need this
right now, but if we decide to add support for this later, having "Cell" in
the name would be unfortunate.

Thanks for the review!
VZ