Re: #14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa

[wxTrac]

#14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa
----------------------+---------------------------
Reporter: dconnet | Owner:
Type: defect | Status: new
Priority: normal | Milestone: 3.2.0
Component: wxOSX | Version: stable-latest
Resolution: | Keywords:
Blocked By: | Blocking:
Patch: 0 |
----------------------+---------------------------

Comment (by oneeyeman):

Possible fix is in the PR 1901 in GitHub.
It makes the behavior the same on all 3 major platforms.

--
Ticket URL: <https://trac.wxwidgets.org/ticket/14939#comment:2>

[wxTrac]

#14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa
----------------------+---------------------------
Reporter: dconnet | Owner:
Type: defect | Status: new
Priority: normal | Milestone: 3.2.0
Component: wxOSX | Version: stable-latest
Resolution: | Keywords:
Blocked By: | Blocking:

Patch: 1 |
----------------------+---------------------------
Changes (by afalkenhahn):

* cc: andreas@… (added)
* patch: 0 => 1


Comment:

Based on my analysis
[https://github.com/wxWidgets/wxWidgets/pull/1901#issuecomment-731574429
here] I have now written a fix for this. In contrast to PR 1901 my patch
is minimally intrusive and tries to use the existing code as far as
possible.

Basically, wxCocoa's dataview backend supports `wxCOL_WIDTH_AUTOSIZE`
already. There are just a few tweaks that had to be applied to make it
work correctly:

1) Column width needs to be re-evaluated whenever items are expanded or
collapsed. My fix does this by calling `AdjustAutosizedColumns()` in the
respective methods now.

2) The `MaxWidthCalculator` class in `dataview.mm` didn't take the size
and offset of the expander icon into account. My fix does this now by
examining the expander's bounding box using NSOutlineView's
`frameOfOutlineCellAtRow`.

3) `MaxWidthCalculator` also wrongly calculated the indent, perhaps this
was a workaround for not taking the expander icon size and offset into
account. This is fixed now too.

I've tested my patch with the dataview sample and my own code and it seems
to work fine. So maybe this issue can finally be closed now.

--
Ticket URL: <https://trac.wxwidgets.org/ticket/14939#comment:3>

[wxTrac]

#14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa
----------------------+---------------------------
Reporter: dconnet | Owner:
Type: defect | Status: new
Priority: normal | Milestone: 3.2.0
Component: wxOSX | Version: stable-latest
Resolution: | Keywords:
Blocked By: | Blocking:
Patch: 1 |
----------------------+---------------------------
Changes (by afalkenhahn):

* Attachment "mypatch.patch" added.

Fix for the issue

--
Ticket URL: <https://trac.wxwidgets.org/ticket/14939>

[wxTrac]

#14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa
----------------------+---------------------------
Reporter: dconnet | Owner:
Type: defect | Status: new

Priority: normal | Milestone: 3.1.5

Component: wxOSX | Version: stable-latest
Resolution: | Keywords:
Blocked By: | Blocking:
Patch: 1 |
----------------------+---------------------------

Changes (by vadz):

* milestone: 3.2.0 => 3.1.5


Comment:

Thank you, this looks good to me. I'd still like to test it myself before
applying, hopefully I'll do it soon when I finally find time to work on
the Mac patches backlog.

@oneeyeman if you see any problems with this patch, could you please
explain how to reproduce them?

--
Ticket URL: <https://trac.wxwidgets.org/ticket/14939#comment:4>

[wxTrac]

#14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa
----------------------+---------------------------
Reporter: dconnet | Owner:

Type: defect | Status: confirmed

Priority: normal | Milestone: 3.1.5
Component: wxOSX | Version: stable-latest
Resolution: | Keywords:
Blocked By: | Blocking:
Patch: 1 |
----------------------+---------------------------
Changes (by vadz):

* status: new => confirmed


Comment:

Thanks again, I've finally tested this and it works for me, so I'm merging
it.

For the reference, here is the updated version of the patch allowing to
reproduce the problem:
{{{
#!diff
diff --git a/samples/dataview/dataview.cpp b/samples/dataview/dataview.cpp
index 98630dc39a..614716ff7e 100644
--- a/samples/dataview/dataview.cpp
+++ b/samples/dataview/dataview.cpp
@@ -775,7 +775,7 @@ void MyFrame::BuildDataViewCtrl(wxPanel* parent,
unsigned int nPanel, unsigned l
wxDataViewTextRenderer *tr =
new wxDataViewTextRenderer( "string",
wxDATAVIEW_CELL_INERT );
wxDataViewColumn *column0 =
- new wxDataViewColumn( "title", tr, 0, FromDIP(200),
wxALIGN_LEFT,
+ new wxDataViewColumn( "title", tr, 0,
wxCOL_WIDTH_AUTOSIZE, wxALIGN_LEFT,
wxDATAVIEW_COL_SORTABLE |
wxDATAVIEW_COL_RESIZABLE );
m_ctrl[Page_Music]->AppendColumn( column0 );
#if 0
@@ -788,7 +788,7 @@ void MyFrame::BuildDataViewCtrl(wxPanel* parent,
unsigned int nPanel, unsigned l

tr = new wxDataViewTextRenderer( "string",
wxDATAVIEW_CELL_EDITABLE );
wxDataViewColumn *column1 =
- new wxDataViewColumn( "artist", tr, 1, FromDIP(150),
wxALIGN_LEFT,
+ new wxDataViewColumn( "artist", tr, 1,
wxCOL_WIDTH_AUTOSIZE, wxALIGN_LEFT,
wxDATAVIEW_COL_SORTABLE |
wxDATAVIEW_COL_REORDERABLE |
wxDATAVIEW_COL_RESIZABLE );
column1->SetMinWidth(FromDIP(150)); // this column can't be
resized to be smaller
@@ -800,7 +800,7 @@ void MyFrame::BuildDataViewCtrl(wxPanel* parent,
unsigned int nPanel, unsigned l
new wxDataViewSpinRenderer( 0, 2010,
wxDATAVIEW_CELL_EDITABLE,
wxALIGN_RIGHT |
wxALIGN_CENTRE_VERTICAL);
wxDataViewColumn *column2 =
- new wxDataViewColumn( "year", sr, 2, FromDIP(60),
wxALIGN_LEFT,
+ new wxDataViewColumn( "year", sr, 2,
wxCOL_WIDTH_AUTOSIZE, wxALIGN_LEFT,
wxDATAVIEW_COL_SORTABLE |
wxDATAVIEW_COL_REORDERABLE );
m_ctrl[Page_Music]->AppendColumn( column2 );

@@ -814,19 +814,19 @@ void MyFrame::BuildDataViewCtrl(wxPanel* parent,
unsigned int nPanel, unsigned l
new wxDataViewChoiceRenderer( choices,
wxDATAVIEW_CELL_EDITABLE,
wxALIGN_RIGHT |
wxALIGN_CENTRE_VERTICAL);
wxDataViewColumn *column3 =
- new wxDataViewColumn( "rating", c, 3, FromDIP(100),
wxALIGN_LEFT,
+ new wxDataViewColumn( "rating", c, 3,
wxCOL_WIDTH_AUTOSIZE, wxALIGN_LEFT,
wxDATAVIEW_COL_REORDERABLE |
wxDATAVIEW_COL_RESIZABLE );
m_ctrl[Page_Music]->AppendColumn( column3 );

// column 4 of the view control:

- m_ctrl[Page_Music]->AppendProgressColumn( "popularity", 4,
wxDATAVIEW_CELL_INERT, FromDIP(80) );
+ m_ctrl[Page_Music]->AppendProgressColumn( "popularity", 4,
wxDATAVIEW_CELL_INERT, wxCOL_WIDTH_AUTOSIZE );

// column 5 of the view control:

MyCustomRenderer *cr = new
MyCustomRenderer(wxDATAVIEW_CELL_ACTIVATABLE);
wxDataViewColumn *column5 =
- new wxDataViewColumn( "custom", cr, 5,
wxCOL_WIDTH_DEFAULT, wxALIGN_LEFT,
+ new wxDataViewColumn( "custom", cr, 5,
wxCOL_WIDTH_AUTOSIZE, wxALIGN_LEFT,
wxDATAVIEW_COL_RESIZABLE );
column5->SetBitmap(wxArtProvider::GetBitmap(wxART_INFORMATION,
wxART_MENU));
m_ctrl[Page_Music]->AppendColumn( column5 );
}}}

It would, of course, still be great to have a unit test checking that this
works, but well...

--
Ticket URL: <https://trac.wxwidgets.org/ticket/14939#comment:5>

[wxTrac]

#14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa
----------------------+---------------------------
Reporter: dconnet | Owner:
Type: defect | Status: confirmed
Priority: normal | Milestone: 3.1.5
Component: wxOSX | Version: stable-latest
Resolution: | Keywords:
Blocked By: | Blocking:
Patch: 1 |
----------------------+---------------------------

Changes (by oneeyeman):

* Attachment "minimal.cpp" added.

Code for test the fix

[wxTrac]

#14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa
----------------------+---------------------------
Reporter: dconnet | Owner:
Type: defect | Status: confirmed
Priority: normal | Milestone: 3.1.5
Component: wxOSX | Version: stable-latest
Resolution: | Keywords:
Blocked By: | Blocking:
Patch: 1 |
----------------------+---------------------------

Comment (by oneeyeman):

Andreas,
Attached is the minimal sample that you can use for testing in addition to
the dataview sample.

In order to see the problem:

{{{
1. Compile and run the sample.
2. In the sample you will see 2 wxDVLC that are layed out vertically.
3. The top one will have some strings and the bottom one will be empty.
4. Start dragging the string from the top one. The row will be added to
the bottom wxDVLC.
5. Observe that the text will be cut off in the column 2.

}}}

With my PR this code works successfully.

I am testing on the OSX 10.13.6 with:


{{{
../configure --enable-debug --with-cocoa --with-cxx=11

}}}

The git shows following:


{{{
Igors-MacBook-Air:minimal igorkorot$ git log
commit 4aafab47e717ea18623b1f8c978e8cb3b35d1b06 (HEAD -> master)
Author: Stefan Csomor <cso...@advancedconcepts.ch>
Date: Sun Oct 25 17:49:37 2020 +0100

Simplify SetFont() in wxOSX implementations

It doesn't need to take the colour and other parameters unused any
more.

Closes https://github.com/wxWidgets/wxWidgets/pull/2103


}}}

Vadim,
I'm sorry for not providing the patch, but rather complete sample code.

I hope Andreas can find the problem and fix it.

And sorry for not providing this code earlier - I was planning to submit
over this weekend, but you caught me with applying this patch prior.

--
Ticket URL: <https://trac.wxwidgets.org/ticket/14939#comment:6>

[wxTrac]

#14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa
----------------------+---------------------------
Reporter: dconnet | Owner:
Type: defect | Status: confirmed
Priority: normal | Milestone: 3.1.5
Component: wxOSX | Version: stable-latest
Resolution: | Keywords:
Blocked By: | Blocking:
Patch: 1 |
----------------------+---------------------------

Comment (by vadz):

I think there is a misunderstanding, the patch hasn't been merged yet,
it's part of https://github.com/wxWidgets/wxWidgets/pull/2136 which is
still open. Please test with this PR.

--
Ticket URL: <https://trac.wxwidgets.org/ticket/14939#comment:7>

[wxTrac]

#14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa
----------------------+---------------------------
Reporter: dconnet | Owner:
Type: defect | Status: confirmed
Priority: normal | Milestone: 3.1.5
Component: wxOSX | Version: stable-latest
Resolution: | Keywords:
Blocked By: | Blocking:
Patch: 1 |
----------------------+---------------------------

Comment (by afalkenhahn):

@Igor: Your minimal sample works correctly with my patch here. But you
should try it for yourself as well, just in case...

--
Ticket URL: <https://trac.wxwidgets.org/ticket/14939#comment:8>

[wxTrac]

#14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa
----------------------+---------------------------
Reporter: dconnet | Owner:
Type: defect | Status: confirmed
Priority: normal | Milestone: 3.1.5
Component: wxOSX | Version: stable-latest
Resolution: | Keywords:
Blocked By: | Blocking:
Patch: 1 |
----------------------+---------------------------

Comment (by oneeyeman):

Andreas,
Replying to [comment:8 afalkenhahn]:

> @Igor: Your minimal sample works correctly with my patch here. But you
should try it for yourself as well, just in case...

I just tried it.

What I did:

1. git clone https://github.com/vadz/wxWidgets/ vadim_fork
2. cd vadim_fork/
3. mkdir buildC11
4. cd buildC11/
5. git submodule update --init
6. git checkout osx-fixes
7. ../configure --enable-debug --enable-debug_gdb --with-cxx=11 --with-
cocoa && make
8. cp ~/wxFork/samples/minimal.cpp ../samples/minimal/
9. cd samples/minimal
10. make && open minimal.app
11. start dragging a string

There is no difference in the result.

I'm testing on OSX 10.13.6 High Sierra with Xcode 9.4.1

@vadz,
Is it the correct procedure?
I actually verified that I have that code - I did "git log | grep -i
col_width_autosize as step 6a.

Unless Vadim somehow missed the portion of the patch.

I will keep Vadim' fork until we sort this out.

I am going to attach a screenshot from me running my code.

Thank you.

--
Ticket URL: <https://trac.wxwidgets.org/ticket/14939#comment:9>

[wxTrac]

#14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa
----------------------+---------------------------
Reporter: dconnet | Owner:
Type: defect | Status: confirmed
Priority: normal | Milestone: 3.1.5
Component: wxOSX | Version: stable-latest
Resolution: | Keywords:
Blocked By: | Blocking:
Patch: 1 |
----------------------+---------------------------

Changes (by oneeyeman):

* Attachment "Screen Shot 2020-12-08 at 1.37.08 AM.png" added.

Screenshot of running the DVC code

[wxTrac]

#14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa
----------------------+---------------------------
Reporter: dconnet | Owner:
Type: defect | Status: confirmed
Priority: normal | Milestone: 3.1.5
Component: wxOSX | Version: stable-latest
Resolution: | Keywords:
Blocked By: | Blocking:
Patch: 1 |
----------------------+---------------------------
Changes (by oneeyeman):

* Attachment "Screen Shot 2020-12-08 at 1.40.52 AM.png" added.

And this is with the 2 strings when second is longer

[wxTrac]

#14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa
----------------------+---------------------------
Reporter: dconnet | Owner:
Type: defect | Status: confirmed
Priority: normal | Milestone: 3.1.5
Component: wxOSX | Version: stable-latest
Resolution: | Keywords:
Blocked By: | Blocking:
Patch: 1 |
----------------------+---------------------------

Comment (by afalkenhahn):

I don't know anything about git so I can't tell if what you did is
correct. All that I can say is that your code works correctly here with my
patch. See my screenshot.

--
Ticket URL: <https://trac.wxwidgets.org/ticket/14939#comment:10>

[wxTrac]

#14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa
----------------------+---------------------------
Reporter: dconnet | Owner:
Type: defect | Status: confirmed
Priority: normal | Milestone: 3.1.5
Component: wxOSX | Version: stable-latest
Resolution: | Keywords:
Blocked By: | Blocking:
Patch: 1 |
----------------------+---------------------------

Changes (by afalkenhahn):

* Attachment "Bildschirmfoto 2020-12-08 um 16.12.08.png" added.

[wxTrac]

#14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa
----------------------+---------------------------
Reporter: dconnet | Owner:
Type: defect | Status: confirmed
Priority: normal | Milestone: 3.1.5
Component: wxOSX | Version: stable-latest
Resolution: | Keywords:
Blocked By: | Blocking:
Patch: 1 |
----------------------+---------------------------

Comment (by vadz):

The commands in comment:9 were clearly retyped manually and not copied
(there are at least a couple of errors in them...) so I don't know what
did you actually do. You must check that you have the commit
e89e76bb82f6d4bd96d1e6943099cd8bfd4e8292 in your local branch.

--
Ticket URL: <https://trac.wxwidgets.org/ticket/14939#comment:11>

[wxTrac]

#14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa
----------------------+---------------------------
Reporter: dconnet | Owner:
Type: defect | Status: confirmed
Priority: normal | Milestone: 3.1.5
Component: wxOSX | Version: stable-latest
Resolution: | Keywords:
Blocked By: | Blocking:
Patch: 1 |
----------------------+---------------------------

Comment (by oneeyeman):

Vadim,
Replying to [comment:11 vadz]:

> The commands in comment:9 were clearly retyped manually and not copied
(there are at least a couple of errors in them...) so I don't know what
did you actually do. You must check that you have the commit
e89e76bb82f6d4bd96d1e6943099cd8bfd4e8292 in your local branch.

Yes, I do:


{{{
Igors-MacBook-Air:minimal igorkorot$ git log | grep
e89e76bb82f6d4bd96d1e6943099cd8bfd4e8292
commit e89e76bb82f6d4bd96d1e6943099cd8bfd4e8292
Igors-MacBook-Air:minimal igorkorot$

}}}

And every command I put in the comment referenced I copy/pasted from the
history.

I would be curious if you can run my sample and and check whose results it
produces.

TIA!

--
Ticket URL: <https://trac.wxwidgets.org/ticket/14939#comment:12>

[wxTrac]

#14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa
----------------------+---------------------------
Reporter: dconnet | Owner:
Type: defect | Status: confirmed
Priority: normal | Milestone: 3.1.5
Component: wxOSX | Version: stable-latest
Resolution: | Keywords:
Blocked By: | Blocking:
Patch: 1 |
----------------------+---------------------------

Comment (by afalkenhahn):

This is unrelated to Igor's problem: Unfortunately, my original patch
needs two additional changes. In the original patch I forgot to adjust the
autosizing columns when the expander is expanded or collapsed by clicking
on it. Obviously, this needs to be done as well so I've now added code to
`outlineViewItemDidCollapse` and `outlineViewItemDidExpand` that does
exactly that.

New patch attached. Please merge.

--
Ticket URL: <https://trac.wxwidgets.org/ticket/14939#comment:13>

[wxTrac]

#14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa
----------------------+---------------------------
Reporter: dconnet | Owner:
Type: defect | Status: confirmed
Priority: normal | Milestone: 3.1.5
Component: wxOSX | Version: stable-latest
Resolution: | Keywords:
Blocked By: | Blocking:
Patch: 1 |
----------------------+---------------------------

Changes (by afalkenhahn):

* Attachment "mypatch_new.patch" added.

[wxTrac]

#14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa

----------------------+-------------------------------------
Reporter: dconnet | Owner: Vadim Zeitlin <vadim@…>
Type: defect | Status: closed

Priority: normal | Milestone: 3.1.5
Component: wxOSX | Version: stable-latest

Resolution: fixed | Keywords:

Blocked By: | Blocking:
Patch: 1 |

----------------------+-------------------------------------
Changes (by Vadim Zeitlin <vadim@…>):

* owner: => Vadim Zeitlin <vadim@…>
* status: confirmed => closed
* resolution: => fixed


Comment:

In [changeset:"e89e76bb82f6d4bd96d1e6943099cd8bfd4e8292/git-wxWidgets"
e89e76bb8/git-wxWidgets]:
{{{
#!CommitTicketReference repository="git-wxWidgets"
revision="e89e76bb82f6d4bd96d1e6943099cd8bfd4e8292"
Make wxCOL_WIDTH_AUTOSIZE work correctly in Mac wxDataViewCtrl

Update the width when items are expanded and collapsed and also take the
expander width into account.

Change m_ModelNotifier type to avoid casts when calling wxOSX-specific
method on it.

Closes #14939.

Co-Authored-By: Vadim Zeitlin <va...@wxwidgets.org>
}}}

--
Ticket URL: <https://trac.wxwidgets.org/ticket/14939#comment:14>

[wxTrac]

#14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa
----------------------+-------------------------------------
Reporter: dconnet | Owner: Vadim Zeitlin <vadim@…>
Type: defect | Status: closed
Priority: normal | Milestone: 3.1.5
Component: wxOSX | Version: stable-latest
Resolution: fixed | Keywords:
Blocked By: | Blocking:
Patch: 1 |
----------------------+-------------------------------------

Comment (by vadz):

Sorry, I can't easily apply this any longer. I've just pushed your
previous patch as part of the merge
938e493456663205fa39897da3e37c4a3bcd3e7e, could you please open a new
ticket with the patch against latest master? TIA!

--
Ticket URL: <https://trac.wxwidgets.org/ticket/14939#comment:15>

[wxTrac]

#14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa
----------------------+-------------------------------------
Reporter: dconnet | Owner: Vadim Zeitlin <vadim@…>
Type: defect | Status: closed
Priority: normal | Milestone: 3.1.5
Component: wxOSX | Version: stable-latest
Resolution: fixed | Keywords:
Blocked By: | Blocking:
Patch: 1 |
----------------------+-------------------------------------

Comment (by afalkenhahn):

Done. See #19003.

--
Ticket URL: <https://trac.wxwidgets.org/ticket/14939#comment:16>

[wxTrac]

#14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa
----------------------+-------------------------------------
Reporter: dconnet | Owner: Vadim Zeitlin <vadim@…>
Type: defect | Status: closed
Priority: normal | Milestone: 3.1.5
Component: wxOSX | Version: stable-latest
Resolution: fixed | Keywords:
Blocked By: | Blocking:
Patch: 1 |
----------------------+-------------------------------------

Comment (by Dimitri Schoolwerth <dimitri@…>):

In [changeset:"9ee866551f98a994a241aa61d87bc8acf94cd2f5/git-wxWidgets"
9ee866551/git-wxWidgets]:
{{{
#!CommitTicketReference repository="git-wxWidgets"
revision="9ee866551f98a994a241aa61d87bc8acf94cd2f5"
Mac: Update column width when expanding/collapsing

Adjust autosizing columns of wxDataViewCtrl when items are expanded
or collapsed through the UI as well.

Complements commit e89e76bb82.

See #14939.

See #19003.
}}}

--
Ticket URL: <https://trac.wxwidgets.org/ticket/14939#comment:17>

[Igor Korot]

Dimitri,

I will try to test my sample over the weekend.

Thank you.

>
> --
> Ticket URL: <https://trac.wxwidgets.org/ticket/14939#comment:17>
>

> --
> To unsubscribe, send email to wx-dev+un...@googlegroups.com
> or visit http://groups.google.com/group/wx-dev
>
> ---
> You received this message because you are subscribed to the Google Groups "wx-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to wx-dev+un...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/wx-dev/061.ffb055ecff361c15a12330dd28bdef04%40wxwiki.org.

[wxTrac]

#14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa
----------------------+-------------------------------------
Reporter: dconnet | Owner: Vadim Zeitlin <vadim@…>

Type: defect | Status: reopened

Priority: normal | Milestone: 3.1.5
Component: wxOSX | Version: stable-latest

Resolution: | Keywords:

Blocked By: | Blocking:
Patch: 1 |
----------------------+-------------------------------------

Changes (by disc):

* status: closed => reopened
* resolution: fixed =>


Comment:

I still have problems using Igor's
[https://github.com/wxWidgets/wxWidgets/pull/1901#issuecomment-744065984
hard to reproduce] sample.

I put up a [https://github.com/discnl/wxWidgets/tree/14939-test-mac-dvc-
autosize branch] with
[https://github.com/discnl/wxWidgets/commit/479c13960ebbb07d79b681c82fb0300d25624bf7.patch
changes] to the dataview sample that should hopefully make it easier to
reproduce (with two columns it always bugs here as long as the last column
has explicit `wxCOL_WIDTH_AUTOSIZE`). When I press "Set Mozart" the last
column does not autosize until I press the button next to it,
"SendSizeEvent()" (i.e. calling `wxDataViewMainWindow::UpdateDisplay()`
for the DVC).

There's also a
[https://github.com/discnl/wxWidgets/commit/383e0e9b9a1e386b2bee47dccf7250ee62f36e12.patch
modified] minimal sample with different tests for two column combinations
and some comments and more quirkiness with an autosizing + default width
column, like Igor's sample (`COL_TEST` 3 in the sample). If the dataview
sample doesn't bug then it's not useful to try this one in an attempt to
reproduce I think.

Finally I had an issue under MSW with autosizing columns not updating
which I first wanted to check to see if it didn't involve common code and
affect this as well. Fixed, as an example, in
[https://github.com/discnl/wxWidgets/commit/6c732ad431503b3cc3fb8d70ebdfc686092c3cb8
6c732ad431] of that branch. While when appending items works because the
entire window is then updated, changing a single cell only updates that
column. I changed it to update autosized columns as well and for that
purpose added `IsAutoSizing()` to, for now only, the generic
`wxDataViewColumn` (I don't think there was something like that already,
the low number of occurrences for `wxCOL_WIDTH_AUTOSIZE` also surprised me
a bit). After now having seen some more wxMSW DVC code, it's hopefully
likely with macOS some things 'just' need synching as well. Let's start
with reproducing the problem first though...

--
Ticket URL: <https://trac.wxwidgets.org/ticket/14939#comment:18>

[Dimitri Schoolwerth]

Hi Igor,

On Wed, 23 Dec 2020 at 02:42, Igor Korot <ikor...@gmail.com> wrote:

 

On Tue, Dec 22, 2020 at 6:48 PM wxTrac <nor...@wxwiki.org> wrote:
>
>  Mac: Update column width when expanding/collapsing
>
>  Adjust autosizing columns of wxDataViewCtrl when items are expanded
>  or collapsed through the UI as well.
>
>  Complements commit e89e76bb82.
>
>  See #14939.
>
>  See #19003.

I will try to test my sample over the weekend.

It is not fixed by this particular commit btw, or any of the recent related ones, if that's what you meant.

Regards,

Dimitri

[Igor Korot]

Dimitri,

Thx for testing.

I am planning to come back to this - maybe over this weekend.
Along with my other PR about bitmap/cursor on OSX.

Thank you.

>
>
> Regards,
> Dimitri

>
> --
> To unsubscribe, send email to wx-dev+un...@googlegroups.com
> or visit http://groups.google.com/group/wx-dev
>
> ---
> You received this message because you are subscribed to the Google Groups "wx-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to wx-dev+un...@googlegroups.com.

> To view this discussion on the web visit https://groups.google.com/d/msgid/wx-dev/CADEMBta_yOzLDdKes%3D5DMrOjd5S-bMuRkFF7NArSsqUn%2Bs5huA%40mail.gmail.com.

[wxTrac]

#14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa
----------------------+-------------------------------------
Reporter: dconnet | Owner: Vadim Zeitlin <vadim@…>
Type: defect | Status: reopened
Priority: normal | Milestone:

Component: wxOSX | Version: stable-latest
Resolution: | Keywords:
Blocked By: | Blocking:
Patch: 1 |
----------------------+-------------------------------------

Changes (by vadz):

* milestone: 3.1.5 =>


Comment:

I don't think it's 3.1.5-critical, so resetting milestone.

--
Ticket URL: <https://trac.wxwidgets.org/ticket/14939#comment:19>

[wxTrac]

#14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa
----------------------+-------------------------------------
Reporter: dconnet | Owner: Vadim Zeitlin <vadim@…>
Type: defect | Status: reopened
Priority: normal | Milestone:
Component: wxOSX | Version: stable-latest
Resolution: | Keywords:
Blocked By: | Blocking:
Patch: 1 |
----------------------+-------------------------------------

Comment (by afalkenhahn):

@vadz: I'm a bit confused now. Hasn't this been merged with the master
already? So it should be part of 3.1.5 or am I misunderstanding something
here?

--
Ticket URL: <https://trac.wxwidgets.org/ticket/14939#comment:20>

[wxTrac]

#14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa
----------------------+-------------------------------------
Reporter: dconnet | Owner: Vadim Zeitlin <vadim@…>
Type: defect | Status: reopened
Priority: normal | Milestone:
Component: wxOSX | Version: stable-latest
Resolution: | Keywords:
Blocked By: | Blocking:
Patch: 1 |
----------------------+-------------------------------------

Comment (by vadz):

I could be missing something here but I've interpreted comment:18 as
saying that there are still some problems left even after these changes. I
admit I haven't actually tested this myself...

--
Ticket URL: <https://trac.wxwidgets.org/ticket/14939#comment:21>

[wxTrac]

#14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa
----------------------+-------------------------------------
Reporter: dconnet | Owner: Vadim Zeitlin <vadim@…>
Type: defect | Status: reopened
Priority: normal | Milestone:
Component: wxOSX | Version: stable-latest
Resolution: | Keywords:
Blocked By: | Blocking:
Patch: 1 |
----------------------+-------------------------------------

Comment (by afalkenhahn):

@vadz: Yes, that's true but still AFAICS the patch is in the master
already so I'd have expected that this means that it'll automatically
become part of 3.1.5.

Anyway, thanks to Dimitri's nice samples I was finally able to reproduce
the issue here and now that I could reproduce it it turned out to be a
really easy fix. I didn't know that the last column should always be made
to fill the whole remaining space, i.e. effectively ignoring
`wxCOL_WIDTH_AUTOSIZE`. This is of course really easy to implement. Check
out the attached patch. The last column will simply be made to fill all
remaining space now. I've tested it with all of Dimitri's samples and they
all work fine so please merge.

This should really be part of 3.1.5 if it isn't yet...

--
Ticket URL: <https://trac.wxwidgets.org/ticket/14939#comment:22>

[wxTrac]

#14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa
----------------------+-------------------------------------
Reporter: dconnet | Owner: Vadim Zeitlin <vadim@…>
Type: defect | Status: reopened
Priority: normal | Milestone:
Component: wxOSX | Version: stable-latest
Resolution: | Keywords:
Blocked By: | Blocking:
Patch: 1 |
----------------------+-------------------------------------

Changes (by afalkenhahn):

* Attachment "mypatch_210126.patch" added.

[wxTrac]

#14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa
----------------------+-------------------------------------
Reporter: dconnet | Owner: Vadim Zeitlin <vadim@…>
Type: defect | Status: reopened
Priority: normal | Milestone:
Component: wxOSX | Version: stable-latest
Resolution: | Keywords:
Blocked By: | Blocking:
Patch: 1 |
----------------------+-------------------------------------

Comment (by vadz):

Thanks, I'll merge it soon and it will be in 3.1.5.

--
Ticket URL: <https://trac.wxwidgets.org/ticket/14939#comment:23>

[wxTrac]

#14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa
----------------------+-------------------------------------
Reporter: dconnet | Owner: Vadim Zeitlin <vadim@…>
Type: defect | Status: reopened
Priority: normal | Milestone:
Component: wxOSX | Version: stable-latest
Resolution: | Keywords:
Blocked By: | Blocking:
Patch: 1 |
----------------------+-------------------------------------

Comment (by Vadim Zeitlin <vadim@…>):

In [changeset:"7555d1b2459949c128d6ee3a473669a1a134f124/git-wxWidgets"
7555d1b24/git-wxWidgets]:
{{{
#!CommitTicketReference repository="git-wxWidgets"
revision="7555d1b2459949c128d6ee3a473669a1a134f124"
Fix expansion of the last column in macOS wxDataViewCtrl

It should always take the rest of the available space.

See #14939.
}}}

--
Ticket URL: <https://trac.wxwidgets.org/ticket/14939#comment:24>

[wxTrac]

#14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa
----------------------+-------------------------------------
Reporter: dconnet | Owner: Vadim Zeitlin <vadim@…>
Type: defect | Status: reopened
Priority: normal | Milestone:
Component: wxOSX | Version: stable-latest
Resolution: | Keywords:
Blocked By: | Blocking:
Patch: 1 |
----------------------+-------------------------------------

Comment (by disc):

Vadim: Sorry about not mentioning the typo in the latest patch (resulting
in a compile error) when I spotted it before.

Thanks for the change Andreas. I can confirm the other tests now work
however `COL_TEST` 3 still bugs here. Good news is I can also reproduce it
in a VM now and am somewhat hopeful it will for you as well then. No
further changes to minimal are needed (just making sure `COL_TEST` 3 is
on):

1. Start the sample
2. Press the left button to set the columns
3. Change the content in the edit control from "abc" to "ab"
4. Press the left button again

For me the right column then slightly shifts to the left.

I can reproduce it in the modified sample e.g. by adding
`m_dataview->SetTextValue("", 0, 0);` after `AppendItem` in `OnButton`, in
which case only the first 2 steps need to be taken. The two changes in
`#if COL_TEST == 3` blocks are also not strictly needed any longer to show
this problem.

I wonder if perhaps something like
[https://github.com/discnl/wxWidgets/commit/6c732ad431503b3cc3fb8d70ebdfc686092c3cb8
6c732ad431] for MSW is again simply needed.

And in case the milestone status stuff wasn't clear: the changes are in
master (3.1.5) but a ticket can also still remain open if it's not fully
solved as is the case here (I would see no reason to also already mention
support for this though).

--
Ticket URL: <https://trac.wxwidgets.org/ticket/14939#comment:25>

[wxTrac]

#14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa
----------------------+-------------------------------------
Reporter: dconnet | Owner: Vadim Zeitlin <vadim@…>
Type: defect | Status: reopened
Priority: normal | Milestone:
Component: wxOSX | Version: stable-latest
Resolution: | Keywords:
Blocked By: | Blocking:
Patch: 1 |
----------------------+-------------------------------------

Comment (by afalkenhahn):

Ok, fixed that now. New patch attached. This patch should also fix the
problem that previously, automatic row height would only work for columns
that were using `wxCOL_WIDTH_AUTOSIZE`. This is no longer the case.
Automatic row height calculation should be working for all columns now
since we call `FitColumnWidthToContent` for all columns now.

--
Ticket URL: <https://trac.wxwidgets.org/ticket/14939#comment:26>

[wxTrac]

#14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa
----------------------+-------------------------------------
Reporter: dconnet | Owner: Vadim Zeitlin <vadim@…>
Type: defect | Status: reopened
Priority: normal | Milestone:
Component: wxOSX | Version: stable-latest
Resolution: | Keywords:
Blocked By: | Blocking:
Patch: 1 |
----------------------+-------------------------------------

Changes (by afalkenhahn):

* Attachment "mypatch_210128.patch" added.

[wxTrac]

#14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa
----------------------+-------------------------------------
Reporter: dconnet | Owner: Vadim Zeitlin <vadim@…>
Type: defect | Status: reopened
Priority: normal | Milestone:
Component: wxOSX | Version: stable-latest
Resolution: | Keywords:
Blocked By: | Blocking:
Patch: 1 |
----------------------+-------------------------------------

Comment (by vadz):

Note to self: need to fix `m_ExpanderWidth` (should be `m_expanderWidth`)
again before applying.

Also, I'm going to close this ticket once the patch is tested and applied,
please don't attach more patches to it, it's getting too confusing, so
let's open new tickets for any new problems. TIA!

--
Ticket URL: <https://trac.wxwidgets.org/ticket/14939#comment:27>

[wxTrac]

#14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa
----------------------+-------------------------------------
Reporter: dconnet | Owner: Vadim Zeitlin <vadim@…>
Type: defect | Status: reopened
Priority: normal | Milestone:
Component: wxOSX | Version: stable-latest
Resolution: | Keywords:
Blocked By: | Blocking:
Patch: 1 |
----------------------+-------------------------------------

Comment (by disc):

Confirmed this fixes that pesky test 3, great! There's a regression (this
time easily reproducible) in master as a result of the previous 210126
patch changes, I have created #19067 for that.

--
Ticket URL: <https://trac.wxwidgets.org/ticket/14939#comment:28>

[wxTrac]

#14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa
----------------------+-------------------------------------
Reporter: dconnet | Owner: Vadim Zeitlin <vadim@…>
Type: defect | Status: reopened
Priority: normal | Milestone:
Component: wxOSX | Version: stable-latest
Resolution: | Keywords:
Blocked By: | Blocking:
Patch: 1 |
----------------------+-------------------------------------

Comment (by vadz):

Just as with #19067, I'm completely lost here because the latest patch
(210128 one) seems to be half-applied: the first chunk is already in
master, but the second one is not. Should I apply the second one? How is
this related to the patch in the other ticket?

I don't want to spend time on redoing the work that you've already done,
but I just don't know what should be done here from just looking at the
code. Could anybody please explain? TIA!

--
Ticket URL: <https://trac.wxwidgets.org/ticket/14939#comment:29>

[wxTrac]

#14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa
----------------------+-------------------------------------
Reporter: dconnet | Owner: Vadim Zeitlin <vadim@…>
Type: defect | Status: reopened
Priority: normal | Milestone:
Component: wxOSX | Version: stable-latest
Resolution: | Keywords:
Blocked By: | Blocking:
Patch: 1 |
----------------------+-------------------------------------

Comment (by afalkenhahn):

Replying to [comment:29 vadz]:

> Just as with #19067, I'm completely lost here because the latest patch
(210128 one) seems to be half-applied: the first chunk is already in
master, but the second one is not.

Also note that even the first chunk seems to have been only half-applied
because in the current master in `dataview.mm` we have these lines:


{{{
if ( pos == noOfColumns - 1 )
[m_OutlineView sizeLastColumnToFit];
else
[column setWidth:calculator.GetMaxWidth() + m_expanderWidth];
}}}

But my patch is slightly different:

{{{
if ( pos == noOfColumns - 1 )
[m_OutlineView sizeLastColumnToFit];
else if ( GetColumn(pos)->GetWidthVariable() == wxCOL_WIDTH_AUTOSIZE )
[column setWidth:calculator.GetMaxWidth() + m_expanderWidth];
}}}

And as you correctly said, the second chunk is not in master at all but
should be applied as well.

To cut a long story short, I think both patches should be applied as is
because AFAIR there are no more issues with them. I have fixed all issues
that were reported.

The discussion in #19067 that started after I submitted my patch was about
the topic that my patch in #19067 is ratcher kludgy but nevertheless it
does the job so should be applied even if it's somewhat ugly to cache the
width to restore it later.

Also, I think Dimitri wanted to apply a certain optimization to the patch
in #19067 but I told him not to do so because I had tried the very same
optimization during the development of the patch in #19067 and it caused
some issues. So the best idea is to just apply the patches from #14939 and
from #19067 as I submitted them.

--
Ticket URL: <https://trac.wxwidgets.org/ticket/14939#comment:30>

[wxTrac]

#14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa
----------------------+-------------------------------------
Reporter: dconnet | Owner: Vadim Zeitlin <vadim@…>
Type: defect | Status: reopened

Priority: normal | Milestone: 3.1.5

Component: wxOSX | Version: stable-latest
Resolution: | Keywords:
Blocked By: | Blocking:
Patch: 1 |
----------------------+-------------------------------------

Changes (by vadz):

* milestone: => 3.1.5


Comment:

Thanks for the explanations! I hope I've applied your patches fully now
but please do check [https://github.com/wxWidgets/wxWidgets/pull/2305 this
PR] and let me know if there is anything still missing, TIA!

For the future, it would be really excellent if you could please open PRs
with your changes instead of having to deal with multitude of patches. And
if you really can't use Git (which IMNSHO is absolutely indispensable),
please let's open new tickets for new patches as having multiple patches
in the same ticket is an excellent recipe for confusion...

FWIW applying PRs would also reduce the turnaround time on my side because
both reviewing with and applying them is much simpler.

--
Ticket URL: <https://trac.wxwidgets.org/ticket/14939#comment:31>

[wxTrac]

#14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa
----------------------+-------------------------------------
Reporter: dconnet | Owner: Vadim Zeitlin <vadim@…>
Type: defect | Status: reopened
Priority: normal | Milestone: 3.1.5
Component: wxOSX | Version: stable-latest
Resolution: | Keywords:
Blocked By: | Blocking:
Patch: 1 |
----------------------+-------------------------------------

Comment (by afalkenhahn):

I've taken a look at your PR and I don't see any problem with it. With
this PR my patches should be complete then and AFAIK there were no other
issues with it after I fixed the ones in #19067.

I'll try to use PRs in the future but normally my patches are rather small
and should be easy to review. I fully agree though that this ticket (and
the ones related to it) have become a mess but hopefully we've got
everything right now :)

--
Ticket URL: <https://trac.wxwidgets.org/ticket/14939#comment:32>

[wxTrac]

#14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa
----------------------+-------------------------------------
Reporter: dconnet | Owner: Vadim Zeitlin <vadim@…>

Type: defect | Status: closed

Priority: normal | Milestone: 3.1.5
Component: wxOSX | Version: stable-latest

Resolution: fixed | Keywords:

Blocked By: | Blocking:
Patch: 1 |
----------------------+-------------------------------------

Changes (by Vadim Zeitlin <vadim@…>):

* status: reopened => closed

* resolution: => fixed


Comment:

In [changeset:"36ea7ff4d61e174a2d116819f98e66f22a7f5b8a/git-wxWidgets"
36ea7ff4d/git-wxWidgets]:
{{{
#!CommitTicketReference repository="git-wxWidgets"
revision="36ea7ff4d61e174a2d116819f98e66f22a7f5b8a"
Autosize the right columns in macOS wxDataViewCtrl

This is related to 7555d1b245 (Fix expansion of the last column in macOS
wxDataViewCtrl, 2021-01-26) which somehow didn't apply the patch fully,
so finish applying it now.

Closes #14939.
}}}

--
Ticket URL: <https://trac.wxwidgets.org/ticket/14939#comment:33>

[wxTrac]

#14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa
----------------------+-------------------------------------
Reporter: dconnet | Owner: Vadim Zeitlin <vadim@…>
Type: defect | Status: closed
Priority: normal | Milestone: 3.1.5
Component: wxOSX | Version: stable-latest
Resolution: fixed | Keywords:
Blocked By: | Blocking:
Patch: 1 |
----------------------+-------------------------------------

Comment (by Vadim Zeitlin <vadim@…>):

In [changeset:"c9221fd538540bd6d105853d0f41620ed25102a5/git-wxWidgets"
c9221fd53/git-wxWidgets]:
{{{
#!CommitTicketReference repository="git-wxWidgets"
revision="c9221fd538540bd6d105853d0f41620ed25102a5"
Fix sizing of temporarily last columns in macOS wxDataViewCtrl

Update the column width if it used to be the last one but isn't last one
any longer.

Closes #14939.
}}}

--
Ticket URL: <https://trac.wxwidgets.org/ticket/14939#comment:34>

[wxTrac]

#14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa
----------------------+-------------------------------------
Reporter: dconnet | Owner: Vadim Zeitlin <vadim@…>
Type: defect | Status: closed
Priority: normal | Milestone: 3.1.5
Component: wxOSX | Version: stable-latest
Resolution: fixed | Keywords:
Blocked By: | Blocking:
Patch: 1 |
----------------------+-------------------------------------

Comment (by Vadim Zeitlin <vadim@…>):

In [changeset:"b66656fbece39f0f9f72b55fbb4745f7b714a4f6/git-wxWidgets"
b66656fbe/git-wxWidgets]:
{{{
#!CommitTicketReference repository="git-wxWidgets"
revision="b66656fbece39f0f9f72b55fbb4745f7b714a4f6"
Merge branch 'dvd-revert-performance-breaking-commits' of
https://github.com/vslavik/wxWidgets

Revert recent changes to wxDataViewCtrl which resulted in unacceptable
performance regressions.

See https://github.com/wxWidgets/wxWidgets/pull/2461

See #14939, #19003.
}}}

--
Ticket URL: <https://trac.wxwidgets.org/ticket/14939#comment:35>

[wxTrac]

#14939: Using wxCOL_WIDTH_AUTOSIZE in wxDataViewColumn does not work under Cocoa
----------------------+-------------------------------------
Reporter: dconnet | Owner: Vadim Zeitlin <vadim@…>

Type: defect | Status: reopened
Priority: normal | Milestone:
Component: wxOSX | Version: stable-latest
Resolution: | Keywords:

Blocked By: | Blocking:
Patch: 1 |
----------------------+-------------------------------------

Changes (by vadz):

* status: closed => reopened
* resolution: fixed =>

* milestone: 3.1.5 =>


Comment:

Unfortunately, this will need to be fixed in a different way to avoid
slowing down the control.

--
Ticket URL: <https://trac.wxwidgets.org/ticket/14939#comment:36>

[Jorge Moraleda]

What is the recommended best practice to work around this issue in the most recent 3.2/3.3 branches? (i.e. to make  `wxCOL_WIDTH_AUTOSIZE` behave under Cocoa the same as in MSW?)

My controls do not have many rows and I can take a performance penalty. I assume the approach will entail Binding some custom method to one or more events to force width recalculation for some columns. But rather than reinventing the wheel, I wonder if someone else has already written this code and would be willing to share it.

Thank you.

Jorge