Skip to content
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.

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

Small Catalogue improvements #5991

Merged

Conversation

johnhaddon
Copy link
Member

  • Allow image properties section (name and description) to be resized.
  • Fix bug that allowed read-only catalogues to be edited.
  • Fix bug when first image added and last image removed.

@johnhaddon johnhaddon self-assigned this Aug 5, 2024
Copy link
Contributor

@murraystevenson murraystevenson left a comment

Choose a reason for hiding this comment

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

LGTM. The SplitContainer does seem preferable to a fixed height description field for anyone stashing render stats in descriptions or the like...

Doing it in `_updateFromValues()` means we don't update at the right times.
These need to track changes to node metadata too. Also, fix editability of `NameWidget( graphComponent = None )`.
The description field was growing a little large for some folks' taste. I did try limiting the description field to a fixed number of lines, but I'm concerned that some other folks might be using large descriptions (lots of different notes on settings perhaps?), so I think providing explicit control is probably better.
In this case, the `index` plug doesn't change value, but whether or not it maps to an actual image does change. And we use that mapping in `_updateFromValues()`, so the first image added wasn't connected to the `__descriptionWidget` and `__nameWidget`, and the last image removed wasn't disconnected.
@johnhaddon johnhaddon merged commit f60e1ea into GafferHQ:1.4_maintenance Aug 7, 2024
6 checks passed
@johnhaddon johnhaddon deleted the catalogueDescriptionHeight branch August 7, 2024 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants