Skip to content

Don't show editScalingRange on another ScaleItem if a draft item already exists#758

Merged
MicroFish91 merged 3 commits intomainfrom
mwf/known-white
Oct 1, 2024
Merged

Don't show editScalingRange on another ScaleItem if a draft item already exists#758
MicroFish91 merged 3 commits intomainfrom
mwf/known-white

Conversation

@MicroFish91
Copy link
Copy Markdown
Contributor

Fix an issue where editScalingRange was showing up on other revision items even when a draft node already existed. In my opinion, it's confusing / doesn't make sense to allow users to try editing other revisions simultaneously when another draft already exists.

@MicroFish91 MicroFish91 requested a review from a team as a code owner September 27, 2024 19:46
@nturinski
Copy link
Copy Markdown
Member

An alternative to this method is to keep showing the command, but throw an error when they try to edit another scale item that isn't the current draft.

My thoughts on why this might be less confusing is because sometimes users don't understand/remember why commands mysteriously disappeared on them. By having the error, it will remind the user that "oh, hey, I do have a current working draft"

@nturinski
Copy link
Copy Markdown
Member

I'm okay with either approach, just wanted to hear thoughts

@MicroFish91
Copy link
Copy Markdown
Contributor Author

Yeah hmm, let me think on this one a bit. I think I initially wanted to do it that way as well but for some reason I decided against it. I'll get back to you

@MicroFish91
Copy link
Copy Markdown
Contributor Author

Couldn't remember a good reason not to do it the way you mentioned, I think it's a better solution as well because it's simpler for us to understand and now we can use these utility functions to easily add the same checks to other template items

@MicroFish91 MicroFish91 merged commit 5e9aa0e into main Oct 1, 2024
@MicroFish91 MicroFish91 deleted the mwf/known-white branch October 1, 2024 22:14
@microsoft microsoft locked and limited conversation to collaborators Nov 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants