search

ArnoldSmith86 / virtualtabletop

a virtual surface in the browser on which you can play board, dice and card games
https://virtualtabletop.io
GNU General Public License v3.0
173 stars 31 forks source link

Formalize use of dropTarget and dropLimit on non-holder widgets #2253

Closed 96LawDawg closed 2 months ago

96LawDawg commented 3 months ago

As recently mentioned in Discord, it is possible to allow cards or any other widget to become a parent just by including dropLimit and dropTarget properties on the widgets that will be the parents. This just adds the default null value in widget.js so the JSON Editor recognizes that and recolors the properties yellow.

The wiki has also already been updated. This has been a hidden feature that we just did not realize existed until today.

ArnoldSmith86 commented 3 months ago

PR-SERVER-BOT: You can play around with it here: https://test.virtualtabletop.io/PR-2253/pr-test (or any other room on that server)

96LawDawg commented 3 months ago

Those do look like substantial changes to the minifier. I wonder if that is what is causing my problem. I opened this PR up, with your changes, on my local machine, but all I get is "Loading room..." I deleted all the room and state and tried again and nothing. But no problem opening other PRs or main. I was going to update a tutorial showing how to do this (with the css overflow), but I can't get my local setup working. Any idea?

RaphaelAlvez commented 3 months ago

Is this PR going two things at once? I see some code that I wouldn't expect needed and the branch name also indicates it has other goals.

96LawDawg commented 3 months ago

I don't think so. The branch name was what I thought the basic fix should be. But it needs to work without forcing a refresh. All the other code does that.

ArnoldSmith86 commented 3 months ago

I opened this PR up, with your changes, on my local machine, but all I get is "Loading room..." I deleted all the room and state and tried again and nothing.

Anything in the console?

96LawDawg commented 3 months ago

Anything in the console?

Uncaught SyntaxError: Identifier 'toServer' has already been declared

ArnoldSmith86 commented 3 months ago

Does it work on the PR test server?

ArnoldSmith86 commented 3 months ago

Does this help?

96LawDawg commented 3 months ago

Does it work on the PR test server?

Yes

Does this help?

No

ArnoldSmith86 commented 3 months ago

Does it work with either "minifyJavascript": false or "minifyJavascript": true in config.json? Can you send me the broken source code (CTRL+U in browser)?

96LawDawg commented 2 months ago

Based on discussion in Discord at https://discord.com/channels/770758631146782780/770758631146782783/1276544777860546713, I reverted @RaphaelAlvez's changes. I think this should be ready to merge. Any final comments from any body?

RaphaelAlvez commented 2 months ago

Based on discussion in Discord at https://discord.com/channels/770758631146782780/770758631146782783/1276544777860546713, I reverted @RaphaelAlvez's changes. I think this should be ready to merge. Any final comments from any body?

Was the property name the only problem?

I think it's least good to incorporate the features where we expect them to be used