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

bpo-37628: Fix IDLE config sample sizes #14958

Merged

Conversation

taleinat
Copy link
Contributor

@taleinat taleinat commented Jul 26, 2019

This fixes the issue where setting a large font size makes the config dialog grow, in some cases making it unusable by pushing the dialog buttons (e.g. "Ok", "Cancel") off-screen.

Tested on Windows 10 and Ubuntu 18.10.

https://bugs.python.org/issue37628

If the normal background is not white, it makes a jagged
border between sample background and following white block.
@terryjreedy
Copy link
Member

I am reviewing and making a few edits now.

Copy link
Member

@terryjreedy terryjreedy left a comment

Choose a reason for hiding this comment

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

Thank you for the quick response. It stops the size explosion issue and with a minor tweak each, the samples look as good as they can without letting the boxes get wider (up to some limit, and at best a future issue). (In reviewing, I found a couple of other issues with the highlight sample that I want to fix tomorrow. along with the key bindings).

My main concern is the addition of two new features (the Text subclass and its decorator and associated code) that seem to me not unneeded here. Until persuaded otherwise, I would like the code simplified, but given the time situation, I am willing to try that in a separate PR.

Lib/idlelib/configdialog.py Outdated Show resolved Hide resolved
Lib/idlelib/configdialog.py Outdated Show resolved Hide resolved
Lib/idlelib/textview.py Outdated Show resolved Hide resolved
Lib/idlelib/textview.py Outdated Show resolved Hide resolved
Lib/idlelib/textview.py Outdated Show resolved Hide resolved
Lib/idlelib/textview.py Outdated Show resolved Hide resolved
Lib/idlelib/textview.py Outdated Show resolved Hide resolved
Lib/idlelib/textview.py Outdated Show resolved Hide resolved
Lib/idlelib/textview.py Outdated Show resolved Hide resolved
Lib/idlelib/textview.py Show resolved Hide resolved
@taleinat
Copy link
Contributor Author

@terryjreedy, I've made the changes you suggested, except for the comment about color_config(); see my reply to that.

@taleinat
Copy link
Contributor Author

FYI, I put up a cleaned version of the generic scrollable text frame as a gist.

Copy link
Member

@terryjreedy terryjreedy left a comment

Choose a reason for hiding this comment

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

Great. I will merge when CI passes after the minor tweaks.

@terryjreedy terryjreedy changed the title bpo-37628: IDLE font config window size bpo-37628: Fix IDLE config sample sizes Jul 27, 2019
@terryjreedy terryjreedy merged commit 3221a63 into python:master Jul 27, 2019
@bedevere-bot
Copy link

@terryjreedy: Please replace # with GH- in the commit message next time. Thanks!

@miss-islington
Copy link
Contributor

Thanks @taleinat for the PR, and @terryjreedy for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7, 3.8.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Sorry @taleinat and @terryjreedy, I had trouble checking out the 3.8 backport branch.
Please backport using cherry_picker on command line.
cherry_picker 3221a63c69268a9362802371a616f49d522a5c4f 3.8

@bedevere-bot
Copy link

GH-14980 is a backport of this pull request to the 3.7 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 27, 2019
The boxes for the font and highlight samples are now constrained by the overall config dialog size.  They gain scrollbars when the when a large font size makes the samples too large for the box.
(cherry picked from commit 3221a63)

Co-authored-by: Tal Einat <taleinat+github@gmail.com>
@miss-islington
Copy link
Contributor

Thanks @taleinat for the PR, and @terryjreedy for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

@bedevere-bot
Copy link

GH-14981 is a backport of this pull request to the 3.8 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 27, 2019
The boxes for the font and highlight samples are now constrained by the overall config dialog size.  They gain scrollbars when the when a large font size makes the samples too large for the box.
(cherry picked from commit 3221a63)

Co-authored-by: Tal Einat <taleinat+github@gmail.com>
miss-islington added a commit that referenced this pull request Jul 27, 2019
The boxes for the font and highlight samples are now constrained by the overall config dialog size.  They gain scrollbars when the when a large font size makes the samples too large for the box.
(cherry picked from commit 3221a63)

Co-authored-by: Tal Einat <taleinat+github@gmail.com>
miss-islington added a commit that referenced this pull request Jul 27, 2019
The boxes for the font and highlight samples are now constrained by the overall config dialog size.  They gain scrollbars when the when a large font size makes the samples too large for the box.
(cherry picked from commit 3221a63)

Co-authored-by: Tal Einat <taleinat+github@gmail.com>
lisroach pushed a commit to lisroach/cpython that referenced this pull request Sep 10, 2019
The boxes for the font and highlight samples are now constrained by the overall config dialog size.  They gain scrollbars when the when a large font size makes the samples too large for the box.
DinoV pushed a commit to DinoV/cpython that referenced this pull request Jan 14, 2020
The boxes for the font and highlight samples are now constrained by the overall config dialog size.  They gain scrollbars when the when a large font size makes the samples too large for the box.
websurfer5 pushed a commit to websurfer5/cpython that referenced this pull request Jul 20, 2020
The boxes for the font and highlight samples are now constrained by the overall config dialog size.  They gain scrollbars when the when a large font size makes the samples too large for the box.
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.

5 participants