Context Menu fixes and improvements #1140
Conversation
cyruzzo#1120 Allow moving context menu to another token. Fix closing popup when notes open.
|
I had to run When using the menu it closes prematurely. Selecting any option, trying to get into a dropdown or moving a slider closes it. We should only close it when clicking outside of it not inside it unless selecting an option we specifically want it to close for. Lots of times I will change multiple options and don't want to have to reopen the menu each time. I agree that opening the menu on a different token or closing on edit note makes sense. Other places where you think it might make sense we can look at too. |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
- Fix bug that would throw error trying to close context menu, if it wasn't there - Close context menu always except for: ContextMenu input field, menu flyout button clicked, any conditions fields, adjustments besides token image, light flyout, options flyout
|
Conflicts have been resolved. A maintainer will review the pull request soon-ish |
Yep, not sure what was going on there. I reverted to back to Load. I can look at moving out later. Also, I think it was easier to close by default, and call out ones we don't want it to close (for the purpose of clicking off the menu etc in case we do something with zindex etc. Just seemed a little easier/safer. The choices in which ones to not close the context menu were just by my feel. Let me know if you disagree with any of them and I will change! |
There was a problem hiding this comment.
I get this error when I right click another token
I added a few comments to the code. Some really good changes in here especially clearing stuff out and letting other things be interactable as you click off the menu to select a token for example. You can just remove the commented out code too I don't think we'll go back.
Token.js
Outdated
| let tinyToken = (Math.round(this.options.gridSquares*2)/2 < 1); | ||
| let addvpps = (!tinyToken || window.CURRENTLY_SELECTED_TOKENS.length > 1) ? parseFloat(window.CURRENT_SCENE_DATA.vpps) : parseFloat(window.CURRENT_SCENE_DATA.vpps)/2; | ||
| let newTop = `${parseFloat(this.options.top) - addvpps}px`; | ||
| moveUp() { |
There was a problem hiding this comment.
Got a few spots of code from an older version trying to remove the new changes.
Token.js
Outdated
|
|
||
|
|
||
|
|
||
| for (let tok of $(".token.tokenselected")){ |
There was a problem hiding this comment.
Hmm yeah im realizing this didn't pick up some of my changes. Ill make those recommendations you had and push the changes that didn't get pushed up
Token.js
Outdated
| let didNotClickAdjustmentsAndTokenImage = () => !!target.parentNode.closest("#adjustments-flyout") && !target.matches("#changeTokenImage"); | ||
| let didNotClickLight = () => !!target.parentNode.closest("#light-flyout"); | ||
| let didNotClickOptions = () => !!target.parentNode.closest("#options-flyout"); | ||
| if (!(clickedMenuInput() || clickedMenuFlyoutButton() || clickedConditions() || didNotClickAdjustmentsAndTokenImage() || didNotClickLight() || didNotClickOptions())) { |
There was a problem hiding this comment.
The hp/max hp and ac inputs closes it still. Aura flyout stuff closes it.
I personally feel the only things it needs to close for are, the group roll, notes and delete.
Maybe a few other buttons I could see being one time clicks (like token image since we're interacting with the side panel after). So basically anything where we're interacting with a new window/area outside the menu immediately after clicking it or removing the token.
I feel it should be consistent where possible and some of the buttons I use in tandem.
I may want to unhide a bunch of monsters while adding them to the combat tracker during an ambush for example. Or add a single monster to the tracker and open it's sheet.
I think the trade off of a click off to close feels better than having to reopen and click for each of those while testing this personally. It could just be my preference though and I'm not sure how it feels using a trackpad or something other than a mouse.
There was a problem hiding this comment.
I think I got everything you asked for. The hiding/unhide feels like we should push them to use the multiselect vs leaving it open and letting them click over right?
There was a problem hiding this comment.
What do you mean by push them to multiselect here? Sorry Im on my phone so I havent been able to test it yet.
The below examples was what I meant when talking about hiding stuff. You may have already changed it this way and your talking about something else.
If i have a group of goblins come out from behind trees abushing the party for example. Id want to unhide their tokens and add them to combat.
If i have a purple worm explode out of the ground I want to be able to do all three, unhide, add to combat and open monster stat block.
I feel like we really dont know when a dm/player might be done with the menu. So in general I feel it should only close when we know they arent going to want to use another button/option.
There was a problem hiding this comment.
Ah I see what you are saying with the combat and open monster stat block piece. Ill make those changes. Thanks for laying that out.
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
|
Conflicts have been resolved. A maintainer will review the pull request soon-ish |
| let addvpps = (!tinyToken || window.CURRENTLY_SELECTED_TOKENS.length > 1) ? parseFloat(window.CURRENT_SCENE_DATA.vpps) : parseFloat(window.CURRENT_SCENE_DATA.vpps)/2; | ||
| let newTop = `${parseFloat(this.options.top) + addvpps}px`; | ||
| let newTop = `${parseFloat(this.options.top) - addvpps}px`; |
There was a problem hiding this comment.
Moving down it looks like this should be + still.
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
#1120 Allow moving context menu to another token. Fix closing popup when notes open.