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

Multiselection and hierarchical Tag entity support #64

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

reformstudios
Copy link

The purpose of this pull request is to allow studios to use the Tag system to browse a media/asset library.

Two major updates to tk-multi-loader2 were required to achieve this.

  • Allow multiselection in the tree-view. (along with various updates to functions to support list items rather than single items)
  • Add support for the 'name' field of 'Tag' entities.

No additional entities are required, but a new field for Tag entities should be added to allow a hierarchy to be formed. The field should be called 'Parent Tag' (or 'sg_parent_tag' in code). It allows you to group tags under other tags to create hierarchies and to organise tags by type or purpose.
For the purpose of a studio library, a new Tag should be created to be the root Tag for the library tag hierarchy; eg 'Library Tag'.

To add a Tag tab to your loader UI, use the following entry in your loader2 config.

entities:
        - caption: Asset Library
          hierarchy: [sg_parent_tag, name]
          entity_type: Tag
          publish_filters:
          - [project, is, {type: Project, id: 90}] # THIS ID SHOULD POINT TO THE PROJECT HOLDING YOUR STUDIO LIBRARY MEDIA/ASSETS.
          filters:
          - [sg_parent_tag.Tag.sg_parent_tag, is, {type: Tag, id: 374}] # THIS ID SHOULD POINT TO THE 'LIBRARY TAG' THAT YOU CREATED PREVIOUSLY.

You can then create a hierarchy of new Tags that define the top groupings of tag for your library. These should all have their parent sent to the 'library tag' tag. The hierarchy can only be 1 level deep currently
eg.

  • Type
    • 2D Element
    • 3D Asset
    • Texture
  • Subject
    • Blood
    • Dust
    • Bubbles
    • Debris
    • MuzzleFlash
  • Style
    • explosion
    • backfire
    • spurt
    • splatter
    • demolition
  • Vehicle
    • Car
    • Bus
    • Spaceship
  • Location
    • London
    • Paris
    • New York
    • Desert
    • Space
      ... etc

The multiselection behaviour in the tree is Toggle behaviour, with each additional item selected narrowing down the number of published items displayed in the right-hand panel. This allows users to use an arbitrary number and type of tags to hone in on relevant search results.

patrick.macdonald added 4 commits February 7, 2018 18:05
Multi entity selection support added.
…l_latestPublish.py.

Various dialog.py functions adapted to support Multiselection in entity tab view.
_populate_entity_breadcrumbs() call moved inside multi-selection item loop.
- legacy "_get_selected_entity()" function removed and replaced with "_get_selected_entities()". This works for single and multiple selection modes.
- _get_selected_entity calls replaced with _get_selected_entities[0] where appropriate.
- Treeview selected logic updated to handle nothing in the selection (refreshes the model) and multiple selections.
model_latestPublish.py:-
- app decleration changed to class member to allow access from other functions. Functions' calls to 'app' replaced with 'self.app'. 'app' only needs to be declared once at __init__
- show_sub_items behaviour was broken by the previous commit. This has now been fixed and performs as expected on both Tag and other entity types.
- self._bundle var removed in favour of the duplicate self.app
@josh-t josh-t self-requested a review May 9, 2018 14:01
Copy link
Contributor

@josh-t josh-t left a comment

Choose a reason for hiding this comment

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

Initial CR pass done. In general it looks pretty solid and super useful. I think the biggest thing is to consider exposing the correlated tag field for the published file as a setting. This would give other clients more flexibility as to how they setup their tagging. Hopefully my comments make sense. Let me know if you have questions.


# now we have arrived at our model derived from StandardItemModel
Copy link
Contributor

Choose a reason for hiding this comment

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

please add these comments back in.

@@ -1242,6 +1249,12 @@ def _load_entity_presets(self):
view.setHeaderHidden(True)
view.setModel(proxy_model)

# Enable multiselection on tag list entities (or in this case extended selection so selections are sticky)
Copy link
Contributor

Choose a reason for hiding this comment

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

the part of the comment in parens is a little confusing. can you clarify?

Copy link
Author

Choose a reason for hiding this comment

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

Sure, multiselect requires shift or ctrl to be pressed. Extended selection doesn't; the selection remains sticky and you can click to toggle items on and off which I thought was a more appropriate UI behaviour for tag filters.

# If nothing is selected, refresh the view.
if len(selected_items) == 0:
if isinstance(model, SgEntityModel):
model.async_refresh()
Copy link
Contributor

Choose a reason for hiding this comment

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

please add a comment on why this is necessary

Copy link
Author

Choose a reason for hiding this comment

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

I could be wrong it's been a while since I looked at this, but if you don't refresh the view when nothing is selected, then you'll be left with a filtered list of published_files, where you actually want the list to reset to fully unfiltered.

model = self._entity_presets[self._current_entity_preset].model
if selected_item and model.canFetchMore(selected_item.index()):
model.fetchMore(selected_item.index())
multi_selection_filters = []
Copy link
Contributor

Choose a reason for hiding this comment

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

this doesn't seemed to be used after it is populated.

# resolve the index into an actual standarditem object
i = self._entity_presets[self._current_entity_preset].model.itemFromIndex(child_idx)
child_folders.append(i)
for item in items:
Copy link
Contributor

Choose a reason for hiding this comment

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

looks like the changes below are strictly the result of adding this line and shifting everything. can you confirm? i don't trust my eyes. 😄

if sg_data:
# leaf node!
# show the items associated. Handle tasks
# via the task field instead of the entity field
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add a period to the end of the old comment here and format the lines a bit for consistency.

# is nothing that you could link up a publish to.
# loop through the selected items
for item in items:
if item is None:
Copy link
Contributor

Choose a reason for hiding this comment

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

checking for a None item here but not in the initial check and show_sub_item loops above. is this necessary? if so, do need to account for it above?

for item in items:
if item is None:
# nothing selected in the treeview
# passing none to _load_data indicates that no query should be executed
Copy link
Contributor

Choose a reason for hiding this comment

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

in the initial check on line 85 above, this was changed to be set to []. Was that necessary? Should this change if so?

# this is an intermediate node like status or asset type which does not
# have any publishes of its own, because the value (e.g. the status or the asset type)
# is nothing that you could link up a publish to.
sg_filters = None
Copy link
Contributor

Choose a reason for hiding this comment

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

same comment here about consistency when this is set to None or []

@@ -536,3 +556,20 @@ def _before_data_processing(self, sg_data_list):
self._publish_type_model.set_active_types( type_id_aggregates )

return new_sg_data

def _log_debug(self, msg):
Copy link
Contributor

Choose a reason for hiding this comment

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

same comment here about whether these are necessary.

@dalorin
Copy link

dalorin commented Nov 27, 2020

Just came across this pull request while trying to implement the exact same functionality. Any chance of this coming back to life and being merged to master?

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.

3 participants