Skip to content

Commit

Permalink
Changed layout of preferences/advanced under GTK
Browse files Browse the repository at this point in the history
The height of wxChoice has been increased for the selection of line endings and also the width of the wxSpinCtrl for the wrap at value.
Otherwise these settings are often not fully visible and can be difficult to set, depending on the environment
  • Loading branch information
c72578 committed Oct 21, 2015
1 parent a370b74 commit 8dca5cb
Showing 1 changed file with 10 additions and 0 deletions.
10 changes: 10 additions & 0 deletions src/prefsdlg.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -898,7 +898,12 @@ class AdvancedPageWindow : public PrefsPanel
auto crlfbox = new wxBoxSizer(wxHORIZONTAL);
sizer->Add(crlfbox, wxSizerFlags().Expand().PXBorder(wxTOP));
crlfbox->Add(new wxStaticText(this, wxID_ANY, _("Line endings:")), wxSizerFlags().Center().BORDER_WIN(wxTOP, PX(1)));
#if defined(__WXGTK__)
// Make the height of wxChoice slightly larger under GTK
m_crlf = new wxChoice(this, wxID_ANY, wxDefaultPosition, wxSize(-1,PX(34)));

This comment has been minimized.

Copy link
@vslavik

vslavik Oct 21, 2015

This is a wrong thing to do, even for GTK+. The height is theme dependent, possibly version dependent, almost certainly GTK+ 2 vs 3 dependent, and user code should never hardcode it like this, in either GTK+ or wx.

This looks like a bug in wxGTK to me. It’s support for GTK+ 3 is still rough around the edges, especially on the 3.0 branch, but improvements are being made, and IIRC I’e seen something related.

It’s worth trying to build against the very latest wxGTK from git (WX_3_0_BRANCH) as well as git master, to see if the bug is fixed.

#else
m_crlf = new wxChoice(this, wxID_ANY);
#endif
m_crlf->Append(_("Unix (recommended)"));
m_crlf->Append(_("Windows"));
crlfbox->Add(m_crlf, wxSizerFlags(1).Center().BORDER_OSX(wxLEFT, PX(3)).BORDER_WIN(wxLEFT, PX(5)));
Expand All @@ -907,7 +912,12 @@ class AdvancedPageWindow : public PrefsPanel
m_wrap = new wxCheckBox(this, wxID_ANY, _("Wrap at:"));
crlfbox->AddSpacer(PX(10));
crlfbox->Add(m_wrap, wxSizerFlags().Center().BORDER_WIN(wxTOP, PX(1)));
#if defined(__WXGTK__)
// Make the width of wxSpinCtrl larger under GTK
m_wrapWidth = new wxSpinCtrl(this, wxID_ANY, "", wxDefaultPosition, wxSize(PX(120),-1));

This comment has been minimized.

Copy link
@vslavik

vslavik Oct 21, 2015

This is clearly something to be fixed in Poedit, but I’m uncomfortable with simply hardcoding another value, which has the same issues as the old value, i.e. not being right in all circumstances.

What would not specifying the size, but specifying a reasonable max value do in your case? I.e. doing this instead:

m_wrapWidth = new wxSpinCtrl(this, wxID_ANY, "888");
#else
m_wrapWidth = new wxSpinCtrl(this, wxID_ANY, "", wxDefaultPosition, wxSize(PX(50),-1));
#endif
m_wrapWidth->SetRange(10, 1000);
crlfbox->Add(m_wrapWidth, wxSizerFlags().Center().BORDER_OSX(wxLEFT, PX(3)));

Expand Down

0 comments on commit 8dca5cb

Please sign in to comment.