Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve the layout of the retracer dialog #882

Merged
merged 3 commits into from Aug 11, 2023
Merged

Improve the layout of the retracer dialog #882

merged 3 commits into from Aug 11, 2023

Conversation

keithel
Copy link

@keithel keithel commented Aug 5, 2023

The retracer dialog had a lot of weird things going on. This should improve it so it looks right on all platforms.

@keithel-qt
Improve the layout of the retracer dialog
548a435 
The retracer dialog had a lot of weird things going on.
This should improve it so it looks right on all platforms.
@okias
Copy link
Contributor

okias commented Aug 5, 2023 •
edited

Nice! Can you share screenshots before and after?

@keithel
Copy link
Author

keithel commented Aug 5, 2023 •
edited

Bah- wish you could set scaling using github's markdown.

Before, without any resizing:
image

Before, resizing the window manually, so you can see the whole content, and see how it expands unintelligently:
image

image

This is on Rocky Linux 8.8 with Gnome 3.32.2, Qt 5.15.14
It still uses the .ui file, even though I really prefer doing it by hand, heh.

@keithel
Copy link
Author

keithel commented Aug 5, 2023

The extra width in the dialog is such that the combobox is wide enough to fit the contents of all entries.
There is some extra width in the combobox beyond that, which I cannot figure out how to get rid of.

okias
okias requested changes Aug 5, 2023
View reviewed changes
Copy link
Contributor

@okias okias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, except one nitpick

gui/ui/retracerdialog.ui Outdated Show resolved Hide resolved
@keithel-qt
Remove setMinimumContentsLength call to show the &->... substitution
88357d3 
To show the & eliding that shouldn't be happening, remove the
setMinimumContentsLength call.
@keithel
Copy link
Author

keithel commented Aug 7, 2023

Ok -seems that the "Run ...Trace" issue I am seeing is a bug that was introduced in Qt 5.15.3, never fixed in Qt 5.15, but fixed in Qt 6 sometime before Qt 6.5.1. I'll just assume this is not an issue.

@keithel-qt
Remove the checkbox layout's left margin
d98d122 
Just go with default margins in the checkbox layout, which is to not
have any margin.

This is in response to okias wanting it to be an odd value (which I had
set to make it positioned pixel-exact with the old approach of using
horizontal spacers).
@keithel keithel requested a review from okias August 7, 2023 17:53
@keithel
Copy link
Author

keithel commented Aug 8, 2023

@okias does this need review from jrfonseca and/or gerddie to be merged?

@okias
Copy link
Contributor

okias commented Aug 11, 2023 •
edited

On my configuration I don't see huge differences, but it looks a bit better then previous window which just has bigger and uglier placement.

Thank you!

@okias okias merged commit 00706d1 into apitrace:master Aug 11, 2023
10 checks passed
@keithel
Copy link
Author

keithel commented Aug 11, 2023

Yes - no radical changes - It was just ugly and some of the content was obscured when expanded or shrunk. Small change, intro to me making changes in this repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@okias okias okias approved these changes

@jrfonseca jrfonseca Awaiting requested review from jrfonseca

@gerddie gerddie Awaiting requested review from gerddie

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@keithel @okias @keithel-qt