Skip to content

Conversation

grliszas14
Copy link
Contributor

@grliszas14 grliszas14 commented Aug 29, 2025

Resolves: #9207
Resolves: #9232
Resolves: #9205
Resolves: #9220
Resolves: #9162

  • I signed CLA
  • The title of the pull request describes an issue it addresses
  • If changes are extensive, then there is a sequence of easily reviewable commits
  • Each commit's message describes its purpose and effects
  • There are no behavior changes unnecessary for the stated purpose of the PR

Recommended:

  • Each commit compiles and runs on my machine without known undesirable changes of behavior

@grliszas14 grliszas14 force-pushed the dynamic_export_options branch from d7f233b to 6351a4c Compare August 29, 2025 07:57
@grliszas14 grliszas14 marked this pull request as ready for review August 29, 2025 07:58
@grliszas14 grliszas14 force-pushed the dynamic_export_options branch 2 times, most recently from 18b116a to 2019200 Compare September 2, 2025 08:14
@grliszas14 grliszas14 force-pushed the dynamic_export_options branch 5 times, most recently from 15c0a01 to 1fe0a64 Compare September 5, 2025 16:22
Component.onCompleted: {
// NOTE: position dialog so entire content is visible
y = -250
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you explain what problem you are solving with this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the export dialog we have static components and dynamically loaded. At the time export dialog is positioned on the screen we only know about static ones -> and it is positioned correctly in the center. But when dynamic components are loaded, the dialog grows in height but the dialog does not reposition itself again: that means a part of the dialog gets rendered below the screen and it's not visible.

This is just a hack to move window a bit more to the top so it doesn't matter which format you choose, you always see entire dialog on the screen.

Do you have any ideas how to solve that riddle without this ugly hack?

@@ -26,4 +26,45 @@ class ExportChannelsPref
};
Q_ENUM(ExportChannels)
};

class ExportOptionType
Copy link
Contributor

Choose a reason for hiding this comment

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

These refer to the UI framework from within an application-logic source file. We had this discussion about the delete behavior (https://mu--se.slack.com/archives/C06RX437GJV/p1755682183400249 - restricted access) and there's a way of keeping this ui-agnostic here and converting to a ui-specific type in a view directory.
This is a bit more work, though, so I'm not sure it's worthwhile. Only if you have some spare energy and you don't find the idea too silly :)

Copy link
Contributor

Choose a reason for hiding this comment

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

If possible I'd really like to reduce all possible fat off of this big, big source file. Things I see possible:

  • Gathering those properties that are used in 6 different place in a dedicated file, as explained in an inline comment.
  • Place the components defined at the bottom of the file in their own source files.
  • Use the RadioButtonGroup with id: channelsGroup properly, making use of its model property (see DeleteBehaviorPanel.qml for an example)
  • Use Row and Column in place of RowLayout and ColumnLayout where possible ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. All of these components are so small after a cleanup that it doesn't really make sense to me to put a separate file with this for example:
Component {
        id: enumComp
        StyledDropdown {
            property var option

            model: option.names
            currentIndex: option.value

            onActivated: function(index, value) {
                dynamicOptionsModel.setData(dynamicOptionsModel.index(option.index, 0),
                                            option.values[index],
                                            ExportOptionType.ValueRole)
            }
        }
    }

left it as is

  1. About RadioButtonGroup: I have it planned to do here: Implement custom mapping for export channels #8963

@grliszas14 grliszas14 force-pushed the dynamic_export_options branch from 1fe0a64 to e0210ab Compare September 9, 2025 07:26
@grliszas14 grliszas14 merged commit 0580af5 into audacity:master Sep 9, 2025
5 checks passed
@grliszas14 grliszas14 deleted the dynamic_export_options branch September 9, 2025 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants