Mix up GTK+ wizard layout a bit #539
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Can one of the admins verify this patch? |
|
@trapas test this please |
|
I like the new layout. |
|
@ernestask will you be making any changes to the code? Because WIP = Work In Progress. |
ernestask
changed the title
No, I just tacked it on as a way to gather input first. |
mkutlak
reviewed
Nov 19, 2018
mkutlak
reviewed
Nov 19, 2018
|
IMO all warnings should be shown in one area (e.g. above the But right now some warnings are shown below the
Glade also reports some deprecated warnings: It would be great if you could fix them aswell. |
For sure, will look into that.
All right. I usually write these by hand, hence me not noticing. |
|
Don’t see anything more spit out by Glade. |
|
The window should no longer expand after maximizing and restoring. |
They’re never used in the code, and don’t make sense if their children are being used as GtkNotebook tabs. Moreover, Glade can display plain boxes just fine. Signed-off-by: Ernestas Kulik <ekulik@redhat.com>
The oriented variants of GtkBox have been deprecated since GTK+ 3.2, and can simply be replaced with GtkBox with optionally-set GtkOrientable:orientation. Signed-off-by: Ernestas Kulik <ekulik@redhat.com>
Signed-off-by: Ernestas Kulik <ekulik@redhat.com>
Currently, the wizard window has a separate box for warnings and action buttons. Given that the warning labels are generally fairly long, they can make the window abnormally large (in conjunction with the homogeneity of GtkNotebook tabs). That can be fixed by showing the warnings on the reporting progress page inline, although that introduces a layout change and inconsistency with other pages. Another issue with the warning labels pertains to their wrapping. Initially, they seem to be slightly underallocated, which results in them overlapping other widgets. That can be worked around by using ellipsization instead. However, a text view might work better, as non-default fonts and their sizes (also some a11y settings) may increase the minimum size of the window at which the labels will be fully readable. https://bugzilla.redhat.com/show_bug.cgi?id=1220158 https://bugzilla.redhat.com/show_bug.cgi?id=1327813 Fixes abrt/abrt#1303 Signed-off-by: Ernestas Kulik <ekulik@redhat.com>
A simple GtkBox will suffice here, plus, GtkTable is deprecated as of GTK+ 3.4: https://developer.gnome.org/gtk3/3.4/GtkTable.html#gtk-table-new Signed-off-by: Ernestas Kulik <ekulik@redhat.com>
These particular check buttons are not expanded, which makes xalign have no effect on the children (labels). Signed-off-by: Ernestas Kulik <ekulik@redhat.com>
GtkStack was introduced with that version. Signed-off-by: Ernestas Kulik <ekulik@redhat.com>
get_event_config() can cause a segfault if NULL is passed to it, since GHashTable tries to hash the pointer as a string. Reproducible just by switching to the “Confirm data to report” tab manually. Signed-off-by: Ernestas Kulik <ekulik@redhat.com>
|
Updated. |
|
LGTM. Thank you @ernestask ! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Smallest possible window size before:
Smallest possible window size after:
This is sort of a spin-off of #538, but also redoes the layout a tad, which may or may not be a big no-no, as it is for desktop applications. Additionally, it adds a scrolled window to the entire page, as opposed to the text view only, which would be quite nice with wrapping enabled on the warning labels, but then we still hit the weird underallocation issues and get text overlapping the revealer. Oh, and word-wrapping for lines in the log text view, to avoid infinite horizontal scrolling.