Skip to content

[GraphEditor] Rewrite AttributeItemDelegate with outside definitions for the components - #3177

Merged
cbentejac merged 26 commits into
developfrom
feat/separatedComponents
Aug 19, 2026
Merged

[GraphEditor] Rewrite AttributeItemDelegate with outside definitions for the components#3177
cbentejac merged 26 commits into
developfrom
feat/separatedComponents

Conversation

@raphaelKoskas

@raphaelKoskas raphaelKoskas commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Description

This PR moves the definition of the components inside AttributeItemDelegate to AttributeControls, as externalised components, thus easing their use in a broader context than the GraphEditor.

Features list

  • Rewrote the components in separated files

Implementation remarks

  • ListAttribute and GroupAttribute have been left as is due to their high dependency on the attribute object.

@codecov

codecov Bot commented Aug 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.27%. Comparing base (643231f) to head (27772a9).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #3177   +/-   ##
========================================
  Coverage    86.27%   86.27%           
========================================
  Files           81       81           
  Lines        12367    12367           
========================================
  Hits         10670    10670           
  Misses        1697     1697           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@raphaelKoskas
raphaelKoskas marked this pull request as ready for review August 7, 2026 10:13

@cbentejac cbentejac left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Components are not, for the most part, fully attribute-agnostic: they either depend on an "attribute" property which is expected to be "shaped" like an Attribute object (which we want to avoid) or they call directly a method that makes attribute-related operations only.

Some components like ColorHue are perfectly instantiated in AttributeItemDelegate, with the slots correctly defined upon instantiation (rather than directly within the component) and this is what we should be doing with the other components as well. (there's just a minor hiccup regarding the direct reference to the "attribute" property, we should only declare and forward the value directly)

The ideal would be to have no mention whatsoever to attributes within the components, as is done in the Choice.qml and ChoiceMulti.qml files.

However the split between components is exactly what we expect (except maybe for the PushButton.qml which does really do anything beside instantiating a standard button) and this heavily improves the readability of AttributeItemDelegate.qml. Most of the hard work has already been done, so kudos on that part!

Comment thread meshroom/ui/qml/GraphEditor/AttributeControls/ColorHue.qml Outdated
Comment thread meshroom/ui/qml/GraphEditor/AttributeControls/ColorHue.qml Outdated
Comment thread meshroom/ui/qml/GraphEditor/AttributeItemDelegate.qml Outdated
Comment thread meshroom/ui/qml/GraphEditor/AttributeControls/PushButton.qml Outdated
Comment thread meshroom/ui/qml/GraphEditor/AttributeItemDelegate.qml Outdated
Comment thread meshroom/ui/qml/GraphEditor/AttributeControls/ColorHue.qml Outdated
Comment thread meshroom/ui/qml/GraphEditor/AttributeControls/ColorHue.qml Outdated
Comment thread meshroom/ui/qml/GraphEditor/AttributeControls/SliderField.qml Outdated
Comment thread meshroom/ui/qml/GraphEditor/AttributeControls/SliderField.qml Outdated
Comment thread meshroom/ui/qml/GraphEditor/AttributeItemDelegate.qml Outdated
@cbentejac cbentejac added this to the Meshroom 2026.1.0 milestone Aug 17, 2026
@cbentejac
cbentejac force-pushed the feat/separatedComponents branch from a3aae09 to d35b1c1 Compare August 18, 2026 09:02
Comment thread meshroom/ui/qml/GraphEditor/AttributeControls/CheckBoxRow.qml Outdated
Comment thread meshroom/ui/qml/GraphEditor/AttributeControls/CheckBoxRow.qml Outdated
@raphaelKoskas
raphaelKoskas force-pushed the feat/separatedComponents branch 4 times, most recently from 9e7c9c3 to 7438401 Compare August 18, 2026 15:48
@raphaelKoskas
raphaelKoskas force-pushed the feat/separatedComponents branch from 7438401 to 70c5e56 Compare August 18, 2026 15:51
@cbentejac
cbentejac force-pushed the feat/separatedComponents branch from 0f07421 to 4c2b683 Compare August 19, 2026 15:21
@cbentejac
cbentejac force-pushed the feat/separatedComponents branch from 4c2b683 to 27772a9 Compare August 19, 2026 15:27
@cbentejac cbentejac changed the title Rewrite AttributeItemDelegate with outside definitions for the components [GraphEditor] Rewrite AttributeItemDelegate with outside definitions for the components Aug 19, 2026
@cbentejac
cbentejac merged commit 8cc5e88 into develop Aug 19, 2026
9 of 10 checks passed
@cbentejac
cbentejac deleted the feat/separatedComponents branch August 19, 2026 15:45
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.

2 participants