Skip to content

Fix: columns rendered with a non-first Selectable widget #761

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
Merged
penguinolog merged 1 commit into from
Jan 22, 2024
Merged

Fix: columns rendered with a non-first Selectable widget #761

penguinolog merged 1 commit into from
Jan 22, 2024

Conversation

@penguinolog
Copy link
Collaborator

@penguinolog penguinolog commented Jan 22, 2024 •
edited
Loading

  • In case of first elements fit, it should be included in the rendered widget.
  • Render columns with FIXED mode should be the same as FLOW mode with cols collected from widget.pack()
Checklist
  • I've ensured that similar functionality has not already been implemented
  • I've ensured that similar functionality has not earlier been proposed and declined
  • I've branched off the master or python-dual-support branch
  • I've merged fresh upstream into my branch recently
  • I've ran tox successfully in local environment
  • I've included docstrings and/or documentation and/or examples for my code (if this is a new feature)

@penguinolog
Fix: columns rendered with a non-first Selectable widget
496b951 
* In case of first elements fit,
  it should be included in the rendered widget.
* Render columns with FIXED mode should be the same as FLOW mode
  with cols collected from `widget.pack()`
@penguinolog penguinolog requested a review from wardi January 22, 2024 14:22
@github-actions github-actions bot added the Tests Tests label Jan 22, 2024
@penguinolog
Copy link
Collaborator Author

penguinolog commented Jan 22, 2024

@wardi this pr fixes old BUG, this is not a regression. Validated, that this bug present in 2.1.2

penguinolog
penguinolog commented Jan 22, 2024
View reviewed changes
urwid/widget/columns.py

elif Sizing.FLOW in w_sizing:
# FIXME: should be able to pack with a different maxcol value
w_size = (maxcol,)
Copy link
Collaborator Author

@penguinolog penguinolog Jan 22, 2024

Choose a reason for hiding this comment

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

here still present part of bug: when we do not know widget size, we assume whole cols and element can be skipped.
Unfortunately, I do not see any good to make universal fix.

@coveralls
Copy link

coveralls commented Jan 22, 2024

Pull Request Test Coverage Report for Build 7612583651

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 70.187%
Totals Coverage Status
Change from base Build 7612182032: 0.0%
Covered Lines: 8349
Relevant Lines: 11985
💛 - Coveralls

@penguinolog penguinolog merged commit dee36d3 into urwid:master Jan 22, 2024
@penguinolog penguinolog deleted the columns_consistency branch January 22, 2024 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wardi wardi wardi approved these changes

Assignees

No one assigned

Labels

bug Tests Tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@penguinolog @coveralls @wardi