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.

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

Fix/dialog width #167

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Fix/dialog width #167

wants to merge 4 commits into from

Conversation

jimfoltz
Copy link
Contributor

Changes size of export dialog and repositions controls a bit to add a little more space.

For some reason, the SKUI dialog has a smaller width in SketchUp 2017 than it did in previous versions. The dialog was cutting off the drop-down controls and pushing the buttons to the left.

Copy link
Member

@thomthom thomthom left a comment

Choose a reason for hiding this comment

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

I just had a quick scan - I need to get back to this one.

:modal => true
}
window = SKUI::Window.new(window_options)
window.background_color = Sketchup::Color.new(240, 240, 240)
Copy link
Member

Choose a reason for hiding this comment

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

I wonder why this is needed. Default SKUI is to use system colors. If the user change system color theme this hard coded value might make things look odd...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It may be a Windows 10 thing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I prefer the contrast to the white background.

Copy link
Member

Choose a reason for hiding this comment

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

Can we deal with the background color in another PR?
At the moment all colors are based on system colors, using system color codes in the CSS stylesheet. I'd rather find a solution where we don't have a mix of hard coded and system colors.

:modal => true
}
window = SKUI::Window.new(window_options)
window.background_color = Sketchup::Color.new(240, 240, 240)
list_width = 160
Copy link
Member

Choose a reason for hiding this comment

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

This makes things look ok in SU2017 as well as older versions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

00083

control.window.close
}
btn_cancel.position(-5, -5)
window.add_control(btn_cancel)

window.default_button = btn_export
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No what does it do? The docs are rather sparse. 😄

Copy link
Member

Choose a reason for hiding this comment

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

You'd think a developer properly document it's code... tsk tsk...

If I recall correctly setting default/cancel buttons will hook up default triggers for Return/ESC keys.

Technically it's unrelated to this CL, can defer to a separate fix.

Copy link
Member

@thomthom thomthom left a comment

Choose a reason for hiding this comment

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

I got one request and that is we take out the hard coded background color and deal with that in a separate PR.

:modal => true
}
window = SKUI::Window.new(window_options)
window.background_color = Sketchup::Color.new(240, 240, 240)
Copy link
Member

Choose a reason for hiding this comment

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

Can we deal with the background color in another PR?
At the moment all colors are based on system colors, using system color codes in the CSS stylesheet. I'd rather find a solution where we don't have a mix of hard coded and system colors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants