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

Itemviews: Introduce new classes Columns & Column, simplfify code #2466

Merged

zas merged 17 commits into metabrainz:master from zas:itemviews_columns

May 13, 2024

Collaborator

zas commented May 12, 2024

Summary

This is a…
- Bug fix
- Feature addition
- Refactoring
- Minor / simple change (like a typo)
- Other
Describe this change in 1-2 sentences:

Problem

JIRA ticket (optional): PICARD-XXX

Solution

Action

Additional actions required:

Update Picard documentation (please include a reference to this PR)
Other (please specify below)

zas requested a review from phw

May 12, 2024 23:06

zas mentioned this pull request

Itemviews refactoring #2454

Closed

7 tasks

zas marked this pull request as ready for review

May 12, 2024 23:17

zas force-pushed the itemviews_columns branch 2 times, most recently from 987ffca to 3c9545d Compare

May 13, 2024 08:50


          Move itemviews.py to itemviews/__init__.py

4fe0b18

zas force-pushed the itemviews_columns branch from 3c9545d to e6a564c Compare

May 13, 2024 11:52

phw previously approved these changes

View reviewed changes

Member

phw left a comment

Good work, this makes the code much more readable and maintainable. Especially with getting rid of all those special cases for certain columns inside the various itemviews functions.

A small question / discussion about the ColumnSortType enum, but otherwise I think this is good to merge.

picard/ui/itemviews/columns.py Outdated Show resolved Hide resolved

zas added 16 commits

May 13, 2024 17:40


          Introduce Columns, Column, IconColumn, and more classes

e06667d


          MainPanel.TITLE_COLUMN -> ITEM_ICON_COLUMN

e2957c3


          Use DEFAULT_COLUMNS, and key/title Column properties

ecf57f6


          Introduce TreeItem.update_colums_text() and reduce code redundancy

9738be7


          Partially replace MainPanel.FINGERPRINT_COLUMN by DEFAULT_COLUMNS.pos…

74e15d4

…('~fingerprint')


          Get header labels from DEFAULT_COLUMNS

a9e7f45


          Move default header formatting outside ConfigurableColumnsHeader

b04d746


          Move column/header resizing code to parent of ConfigurableColumnsHeader

a05a90f


          Ensure first column is always visible

c7da29c


          Use IconColumn.paint_icon()

5fe6963


          on_sort_indicator_changed(): this applies to all icon columns, not on…

c4c89fa

…ly fingerprint


          Use Column properties, get rid of a bunch of MainPanel references

38893b0


          Use ColumnSortType and Column.sortkey(), get rid of few more MainPane…

5d4bca1

…l references


          Drop (now) unused MainPanel columns-related properties

393e677


          ConfigurableColumnsHeader: No point in calling setSectionsMovable/Cli…

4dc268c

…ckable

Those will be called anyway in TristateSortHeaderView.lock() called from TristateSortHeaderView.__init__()


          show_column(): minor simplification using set.discard() instead of …

03914b2

…`set.remove()`

zas dismissed phw’s stale review via

03914b2

May 13, 2024 15:40

zas force-pushed the itemviews_columns branch from e6a564c to 03914b2 Compare

May 13, 2024 15:40

zas requested a review from phw

May 13, 2024 15:43

phw approved these changes

View reviewed changes

zas merged commit 4fe15e0 into metabrainz:master

45 checks passed

zas deleted the itemviews_columns branch

May 13, 2024 15:50

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