Kees (PR #25546)
Kees van der Oord (Nikon)
fixes for two flaws:
- in grid.cpp fix mix up of wxALIGN_CENTER, wxALIGN_CENTER_VERTICAL and wxALIGN_CENTER_HORIZONTAL
- in htmlpars.cpp added support for HTML charset attribute
You can view, comment on, or merge this pull request online at:
https://github.com/wxWidgets/wxWidgets/pull/25546
Commit Summary
- 5a0dd0b Update grid.cpp
- e347af4 Update htmlpars.cpp
- e406216 Merge branch 'master' of https://github.com/Kees-van-der-Oord-Nikon/wxWidgets
File Changes
(2 files)
- M src/generic/grid.cpp (4)
- M src/html/htmlpars.cpp (8)
Patch Links:
- https://github.com/wxWidgets/wxWidgets/pull/25546.patch
- https://github.com/wxWidgets/wxWidgets/pull/25546.diff
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#25546)Thanks for the fixes, but could you please explain how to reproduce the bug fixed by the first commit (in wxGrid)? It doesn't seem that obvious to me which alignment constant should be used here, so I'd really like to test the fix before merging it.
TIA!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Kees van der Oord (Nikon)
@Kees-van-der-Oord-Nikon pushed 1 commit.
- e5bb9b3 Update grid.cpp
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Kees van der Oord (Nikon)
Kees-van-der-Oord-Nikon left a comment (wxWidgets/wxWidgets#25546)Dear Vadz,
I fixed this in a rather large project that include the xlnt library to load .xlsx files, it would take me considerable time to make a minimal example. I hope the arguments below are sufficient for you to understand the details.
When calling wxGrid::DrawTextRectangle(dc,text,wxALIGN_CENTER_HORIZONTAL,wxALIGN_CENTER_VERTICAL,0),
the current implementation does not align the text correctly. The first pull request fixed that but it broke compatibility with the situation that the function is called like this:
wxGrid::DrawTextRectangle(dc,text,wxALIGN_CENTER,wxALIGN_CENTER,0);
I just pushed a second pull request that restores the original case line and only adds case lines to support wxALIGN_CENTER_HORIZONTAL and wxALIGN_CENTER_VERTICAL.
WR,
Kees
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#25546)The problem is that it's really not clear whether the parameter should be wxALIGN_CENTRE or wxALIGN_CENTRE_{HORIZONTAL,VERTICAL}. Accepting both of them should be fine, but perhaps it would be even better to change the caller using the latter constants to use wxALIGN_CENTRE instead?
How/when exactly does this function end up being called with wxALIGN_CENTRE_{HORIZONTAL,VERTICAL} in your application?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#25546)The library that parses the xlsx returns perfectly correct values for the horizontal and vertical alignment: Horizontal: wxALIGN_LEFT, wxALIGN_CENTER_HORIZONTAL, wxALIGN_RIGHT Vertical: wxALIGN_TOP, wxALIGN_CENTER_VERTICAL, wxALIGN_BOTTOM That the constant wxALIGN_CENTER is defined as a convenient value to indicate both horizontal and vertical alignment is an extra feature, on top of the basics above. IMHO, the function wxGrid::DrawTextRectangle() should at least support the basic, on top of that also handle the case when the align parameter also has irrelevant bits set.
But in wxGrid code only wxALIGN_CENTRE is used... DrawTextRectangle() is unfortunately not documented, so it's not clear what its parameters are required to be, but it seems that it really expects wxALIGN_CENTRE.
Anyhow, it should probably be safe to apply this, even if it does make things rather messy.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.