Skip to content

Comments

Add input parameters to the toolbox in the modeler#61187

Merged
nyalldawson merged 19 commits intoqgis:masterfrom
ValentinBuira:unify-list-for-modeler
Apr 29, 2025
Merged

Add input parameters to the toolbox in the modeler#61187
nyalldawson merged 19 commits intoqgis:masterfrom
ValentinBuira:unify-list-for-modeler

Conversation

@ValentinBuira
Copy link
Contributor

@ValentinBuira ValentinBuira commented Mar 25, 2025

Description

This PR create a single "Toolbox" panel for both the Inputs and Algorithms panel under a single panel.

The "Algorithms" panel is turned into a "Toolbox" panel and is embellished with a new sub categories "Input parameters" that is now searchable along side other model component. The parameter inside the toolbox behave as you would expect with double click and drag and drop to add it to the model

This feature avoid the annoyance to switch between the two panels back and forth. Additionally the model parameters are now searchable as well !

To note: The "Inputs" panel is still available on his own

Thanks to the Hauts-de-France region for sponsorising this work and Camptocamp for their collaboration

Before the PR:
image

After the PR:

Screencast.From.2025-03-25.17-15-06.webm

@github-actions
Copy link
Contributor

github-actions bot commented Mar 25, 2025

🪟 Windows Qt6 builds

Download Windows Qt6 builds of this PR for testing.
(Built from commit af6fd23)

🪟 Windows builds

Download Windows builds of this PR for testing.
Debug symbols for this build are available here.
(Built from commit af6fd23)

@github-actions github-actions bot added this to the 3.44.0 milestone Mar 25, 2025
@nirvn
Copy link
Contributor

nirvn commented Mar 26, 2025

@ValentinBuira , hey there, I think it's the first time I'm commenting on a PR of yours, nice to meet your code! ;)

On this PR, I'm -1 to remove the algorithm and parameters panels. I have a fairly large screen, and I much prefer to have both panels opened side by side (one above the other) and keep these separated.

Now, I can also see an argument for people who want things merged together (small screen size, or just a subjective preference). What I'd propose here is for you to keep the alg. and parameters panels as is, and add a new panel (maybe call it toolbox, or components) that will have a tree structure like this:

- Parameters
  - Vector layer
  - Vector feature
  - etc...
- Algorithms
  - Vector creation
    - Random points
    - Random lines
    - etc...
  - Vector analysis
    - Difference
    - Union
    - etc...
  - etc...

Having categories as root notes makes for a cleaner presentation and allows for more components / types to be added in the future (say we have conditional nodes for e.g.).

I'm not against finding a way forward on this, and I very much appreciate your efforts. We just have to make sure it will be satisfactory for all and future proof :)

@ValentinBuira
Copy link
Contributor Author

ValentinBuira commented Mar 26, 2025

Hi @nirvn thanks for your feedback

The design is based of the Google Summer of code proposal I did last year, and that we discussed on the mailling list.

As it stand, I am not that keen to duplicate panel that would be so similar in functionalities. In this PR I have been mostly following existing practice within the modeler for example:

Having categories as root notes makes for a cleaner presentation and allows for more components / types to be added in the future (say we have conditional nodes for e.g.).

In fact this already exist with the "Modeler tools" and is integrated in the algorithm panel as well. (It's even a bit more hidden as the icon is the same as algorithm)
modeler tool

I don't know what do you think about it @nyalldawson ?

@nyalldawson
Copy link
Collaborator

I tend towards @nirvn's opinion that this isn't a universal "win". It will definitely improve some workflows, but I do think it comes at the cost of a bit less user-friendliness. Users will have to seek out the "model parameters" group, whereas the existing approach just puts these right in their faces. So in my view it's a win for some experienced users, but a step backwards for QGIS beginners.

That's why I'd personally like something like @nirvn's proposal. We add the "model parameters" group to the algorithms list, rename this as "toolbox", but keep the existing "inputs" panel available (and visible by default). We then remember the state of this panel, so if a user doesn't want it they can close it and it'll be closed for all future sessions.

@nyalldawson
Copy link
Collaborator

Let's bring @alexbruy , @andreasneumann , and @DelazJ into the conversation too -- they'll all have valuable feedback.

else if ( isFavoriteNode )
return tr( "Favorites" );
else if ( isParameterGroupNode )
return tr( "Model parameters" );
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure about wording here, maybe it would be better to leave it as "Inputs" or "Algorithm parameters"?

Copy link
Contributor Author

@ValentinBuira ValentinBuira Apr 3, 2025

Choose a reason for hiding this comment

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

I went for the middle ground "Input parameters", so it inherit both the name "Inputs" from the initial panel and "parameters" from the first node item in this panel

@alexbruy
Copy link
Contributor

I also think that we should keep existing "Inputs" panel available at least for some time. While having a single toolbox with inputs and algorithms can be more comfortable in some cases, in other cases having a separate panel is more useful.

@ValentinBuira
Copy link
Contributor Author

ValentinBuira commented Apr 3, 2025

I understand than the consensus is to keep the two panels at least in some form.

So, I made a synthesis of your comments:

  • The "Inputs" panel would be kept as is, and the reorder inputs button still belong to this panel
  • The algorithms panel is turned into a "Toolbox" panel and is embellished with a new sub categories "Input parameters" that is now searchable along side other model component. The parameter inside the toolbox behave as you would expect with double click and drag and drop to add it to the model

See at the bottom of the for a screenshot of the current state

If the design is settled then it's ready for a proper review.

image

@nyalldawson
Copy link
Collaborator

Design looks good to me (except "inputS parameters" in the tree should be just "input parameters")!

@nirvn thoughts?

@ValentinBuira
Copy link
Contributor Author

except "inputS parameters"

Englishing is hard sometimes :-) Fixed thanks

@github-actions
Copy link
Contributor

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

  • that all unit tests are passing

  • that all comments by reviewers have been addressed

  • that there is enough information for reviewers, in particular

    • link to any issues which this pull request fixes

    • add a description of workflows which this pull request fixes

    • add screenshots if applicable

  • that you have written unit tests where possible
    In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
    If there is no further activity on this pull request, it will be closed in a week.

@github-actions github-actions bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Apr 22, 2025
@ValentinBuira
Copy link
Contributor Author

This PR is still active and waiting for a review

Ping @nirvn, do you think you could have a second look at the design ?

@github-actions github-actions bot removed the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Apr 22, 2025
@nirvn
Copy link
Contributor

nirvn commented Apr 28, 2025

Looking good to me, thanks for being responsive to our requests @ValentinBuira

@ValentinBuira
Copy link
Contributor Author

ValentinBuira commented Apr 29, 2025

I corrected the last comments review, and it's ready to be merged, thanks all for your involvement

@nyalldawson nyalldawson merged commit 2b51d04 into qgis:master Apr 29, 2025
31 checks passed
@DelazJ
Copy link
Contributor

DelazJ commented Apr 30, 2025

@ValentinBuira Thanks for your contribution. Can you double-check whether the top message is inline with the final implementation so that we could appropriately label the PR for changelog and documentation, please? Same applies to #60664. Thanks

@ValentinBuira ValentinBuira changed the title Add unified panel for input or algorithm in the modeler Add input parameters to the toolbox in the modeler May 2, 2025
@ValentinBuira
Copy link
Contributor Author

@ValentinBuira Thanks for your contribution. Can you double-check whether the top message is inline with the final implementation

Thanks @DelazJ, I have updated the PR description to match the end implementation. And for #60664 it stayed the same

@DelazJ DelazJ added Needs Documentation When merging a labeled PR, an issue will be created in the Doc repo. Changelog Items that are queued to appear in the visual changelog - remove after harvesting labels May 4, 2025
@qgis-bot
Copy link
Collaborator

qgis-bot commented May 4, 2025

@ValentinBuira
This pull request has been tagged as requiring documentation.

A documentation ticket will be opened at https://github.com/qgis/QGIS-Documentation when this PR is merged.

Please update the description (not the comments) with helpful description and screenshot to help the work from documentors.
Also, any commit having [needs-doc] or [Needs Documentation] in will see its message pushed to the issue, so please be as verbose as you can.

Thank you!

@qgis-bot
Copy link
Collaborator

qgis-bot commented May 4, 2025

@ValentinBuira

This pull request has been tagged for the changelog.

  • The description will be harvested so please provide a "nearly-ready" text for the final changelog
  • If possible, add a nice illustration of the feature. Only the first one in the description will be harvested (GIF accepted as well)
  • If you can, it's better to give credits to your sponsor, see below for different formats.

You can edit the description.

Format available for credits
  • Funded by NAME
  • Funded by URL
  • Funded by NAME URL
  • Sponsored by NAME
  • Sponsored by URL
  • Sponsored by NAME URL

Thank you!

@qgis-bot
Copy link
Collaborator

qgis-bot commented May 4, 2025

@ValentinBuira
A documentation ticket has been opened at qgis/QGIS-Documentation#9848
It is your responsibility to visit this ticket and add as much detail as possible for the documentation team to correctly document this change.
Thank you!

@zacharlie zacharlie added ChangelogHarvested This PR description has been harvested in the Changelog already. and removed Changelog Items that are queued to appear in the visual changelog - remove after harvesting labels May 7, 2025
@ValentinBuira ValentinBuira deleted the unify-list-for-modeler branch August 26, 2025 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ChangelogHarvested This PR description has been harvested in the Changelog already. Needs Documentation When merging a labeled PR, an issue will be created in the Doc repo.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants