Asset Pipeline: Improve Push Operator UI #199

Merged
Member

What's Changed?

  • Remove Save Bool (because Save is required for Push)
  • Expose Pull before Push Option in non-advanced Mode
  • Rename Push Operator to Push & Pull
  • Update Push & Pull Tooltip
  • Enforce Black Formatting

Updated UI

image

## What's Changed? - Remove Save Bool (because Save is required for Push) - Expose Pull before Push Option in non-advanced Mode - Rename Push Operator to Push & Pull - Update Push & Pull Tooltip - Enforce Black Formatting ## Updated UI ![image](/attachments/c9925931-e54a-4ba9-a119-13e976548ad2)
Nick Alberelli added 6 commits 2024-01-10 16:09:56 +01:00
Nick Alberelli requested review from Simon Thommes 2024-01-10 16:10:05 +01:00
Nick Alberelli requested review from Demeter Dzadik 2024-01-10 16:10:05 +01:00
Member

What I meant was actually to have a fully separate button for pushing only instead of the toggle. So 3 Separate buttons:

  • Push & Pull
  • Push
  • Pull

But I'm fine with waiting for Met to chime in and have that signed off before doing anything.

What I meant was actually to have a fully separate button for pushing only instead of the toggle. So 3 Separate buttons: - Push & Pull - Push - Pull But I'm fine with waiting for Met to chime in and have that signed off before doing anything.
Member

I'm fine with 3 separate buttons if that means we can get rid of the pop-up, but I would only expose the "Push & Pray" (without pull) as an "advanced" option, so rest of the art team that doesn't have a strong understanding of it has a lower chance to misuse it and cause fires.

I'm fine with 3 separate buttons if that means we can get rid of the pop-up, but I would only expose the "Push & Pray" (without pull) as an "advanced" option, so rest of the art team that doesn't have a strong understanding of it has a lower chance to misuse it and cause fires.
Author
Member

Personally I am strongly opposed to have the Push Only aka Push & Pray button as it doesn't align with the software's design. The add-on assumes the user must have pulled before they pushed and skipping that step for the average user can result in sync conflics.

I am more in favor of @Mets suggestion that we hide Push & Pray behind the advanced menu, it is essentially the same thing as having the Pull before Push bool just in a different UI.

Personally I am strongly opposed to have the Push Only aka Push & Pray button as it doesn't align with the software's design. The add-on assumes the user must have pulled before they pushed and skipping that step for the average user can result in sync conflics. I am more in favor of @Mets suggestion that we hide Push & Pray behind the advanced menu, it is essentially the same thing as having the `Pull before Push` bool just in a different UI.
Member

Then we should probably have another call about this and potentially involve other people's opinions since we talked about this before and I'm still very much in favor of making this button available just as well as the other ones and respecting the user's ability to make an intelligent decision about which workflow to go for.

Manually pressing the pull and push button individually is just as viable as the single button to do both in one step. There is absolutely no difference if you don't update the svn checkout in between those steps. I don't understand how that would be any less compatible with the software's design.

After a major change that could potentially cause issues I prefer to pull first, then see if something broke and push after that. Rather than pressing the pull and push button, hoping that things go right, see that something broke after I waited twice the time to find that out and having to revert the publish.

If we can ever eliminate the tangible cost of the pull process whenever it is redundat, I'm all for hiding that button behind an advanced mode. But other than that I am strongly in favor of presenting the user with all, clearly defined options, giving the tools to debug when something goes wrong and guiding the intended way to use it with minimal problems on a UI level.

Then we should probably have another call about this and potentially involve other people's opinions since we talked about this before and I'm still very much in favor of making this button available just as well as the other ones and respecting the user's ability to make an intelligent decision about which workflow to go for. Manually pressing the pull and push button individually is just as viable as the single button to do both in one step. There is absolutely no difference if you don't update the svn checkout in between those steps. I don't understand how that would be any less compatible with the software's design. After a major change that could potentially cause issues I prefer to pull first, then see if something broke and push after that. Rather than pressing the pull and push button, hoping that things go right, see that something broke after I waited twice the time to find that out and having to revert the publish. If we can ever eliminate the tangible cost of the pull process whenever it is redundat, I'm all for hiding that button behind an advanced mode. But other than that I am strongly in favor of presenting the user with all, clearly defined options, giving the tools to debug when something goes wrong and guiding the intended way to use it with minimal problems on a UI level.
Member

Let's just do it and if it results in misuse, we can change it if needed.

We can also suggest that doing just Push needs extra care by labeling the buttons accordingly.
1: "Pull"
2: "Pull & Push" / "Sync & Push" / "Sync" / not "Push & Pull" even though it sounds better, because that'd lie about the order.
3: "Push Without Sync" / "Push Without Pull" / "Only Push" / anything but just "Push".

(Keep the current "to/from active/staged" bit at the end ofc.)

Let's just do it and if it results in misuse, we can change it if needed. We can also suggest that doing just Push needs extra care by labeling the buttons accordingly. 1: "Pull" 2: "Pull & Push" / "Sync & Push" / "Sync" / not "Push & Pull" even though it sounds better, because that'd lie about the order. 3: "Push Without Sync" / "Push Without Pull" / "Only Push" / anything but just "Push". (Keep the current "to/from active/staged" bit at the end ofc.)
Author
Member

Ok so to @SimonThommes point, the real issue here is the speed of syncing, and the solution to that, to reduce the wait time is to complete the High Priority task to actually improve performance. But I accept that this won't happen tomorrow so we can focus on improving the UX based on ur guy's feedback. As long as the two of you agree, I can execute it.

@Mets I don't understand what labels you are suggesting based on your message, or are you providing mulitple options? Below are the labels I think make most sense... and just expose all three without advanced mode, I will add a UI element to the Push Operator's pop up warning the user they can mess up the process if they have not pulled first.

  • Pull
  • Sync
  • Push
Ok so to @SimonThommes point, the real issue here is the speed of syncing, and the solution to that, to reduce the wait time is to complete the [High Priority task to actually improve performance](https://projects.blender.org/studio/blender-studio-pipeline/issues/170#:~:text=Improve%20performance%20by%20using%20classes%20to%20organize%20transferable%20data). But I accept that this won't happen tomorrow so we can focus on improving the UX based on ur guy's feedback. As long as the two of you agree, I can execute it. @Mets I don't understand what labels you are suggesting based on your message, or are you providing mulitple options? Below are the labels I think make most sense... and just expose all three **without** advanced mode, I will add a UI element to the Push Operator's pop up warning the user they can mess up the process if they have not pulled first. - Pull - Sync - Push
Member

Yeah, I was suggesting different options. I'd not label it just "Push" because that doesn't communicate that it's something that should be used with care. But I also don't mean to make it too wordy either. How about "Force Push"?

Yeah, I was suggesting different options. I'd not label it just "Push" because that doesn't communicate that it's something that should be used with care. But I also don't mean to make it too wordy either. How about "Force Push"?
Author
Member

Yeah, I was suggesting different options. I'd not label it just "Push" because that doesn't communicate that it's something that should be used with care. But I also don't mean to make it too wordy either. How about "Force Push"?

"Force Push" Sounds Good To me

> Yeah, I was suggesting different options. I'd not label it just "Push" because that doesn't communicate that it's something that should be used with care. But I also don't mean to make it too wordy either. How about "Force Push"? "Force Push" Sounds Good To me
Member

Sure, sounds good 👍

Sure, sounds good 👍
Nick Alberelli added 1 commit 2024-01-12 22:09:20 +01:00
Author
Member

Alright I have updated the UI, please see the image below and either Approve the PR using the Review button on the Files Changed page if you agree or leave more feedback.

Below is the Operator Pop-Up when Force Pushing, the Operator Pop-Up for Sync and Pull remains the same as was before.

image

Alright I have updated the UI, please see the image below and either [Approve the PR](https://projects.blender.org/studio/blender-studio-pipeline/pulls/199/files) using the Review button on the Files Changed page if you agree or leave more feedback. Below is the Operator Pop-Up when Force Pushing, the Operator Pop-Up for Sync and Pull remains the same as was before. ![image](/attachments/c77eed79-2cd4-4939-b877-7373985e498d)
Simon Thommes requested changes 2024-01-15 10:24:54 +01:00
Simon Thommes left a comment
Member

Thanks! I'd just reverse the order of the three buttons. Otherwise looks good to me

Thanks! I'd just reverse the order of the three buttons. Otherwise looks good to me
Member

Just to make sure: The sync button has not bool properties left now , correct? Since saving isn't optional and pushing without saving has its own button now.

Just to make sure: The sync button has not bool properties left now , correct? Since saving isn't optional and pushing without saving has its own button now.
Author
Member

@SimonThommes correct, there is no Bool in either the Sync nor the Force Push Operators (pictured above)

Sync Operator Pop-Up

image

They are actually the same operator I am just drawing the operator with the bool already enabled (for sync) and not enabled (force push). See the changes to ui.py for more info.

@SimonThommes correct, there is no Bool in either the `Sync` nor the `Force Push` Operators (pictured above) ### Sync Operator Pop-Up ![image](/attachments/f9886f4d-555c-4e61-85c9-779932842dbf) They are actually the same operator I am just drawing the operator with the bool already enabled (for sync) and not enabled (force push). See the changes to [`ui.py`](https://projects.blender.org/studio/blender-studio-pipeline/pulls/199/files#diff-0f30888c2741b7b67043151d077027b531f0ed43) for more info.
Nick Alberelli requested review from Simon Thommes 2024-01-15 16:04:16 +01:00
Nick Alberelli added 1 commit 2024-01-16 17:44:31 +01:00
Author
Member

Thanks! I'd just reverse the order of the three buttons. Otherwise looks good to me

image

> Thanks! I'd just reverse the order of the three buttons. Otherwise looks good to me ![image](/attachments/e7342ed8-fc84-4752-9c19-97fcc09722d1)
Simon Thommes approved these changes 2024-01-16 18:29:58 +01:00
Simon Thommes left a comment
Member

Didn't test the functionality, but looks good now, thanks 👍

Didn't test the functionality, but looks good now, thanks 👍
Nick Alberelli merged commit 888b72a50e into main 2024-01-16 18:32:12 +01:00
Nick Alberelli deleted branch feature/asset-pipeline-update-sync-ui 2024-01-16 18:32:12 +01:00
Sign in to join this conversation.
No Milestone
No project
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: studio/blender-studio-tools#199
No description provided.