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

V2Application Compatability Changes #130

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

WBHarry
Copy link

@WBHarry WBHarry commented Aug 28, 2024

Heya! I haven't really used this module before, but a user of the module I'm currently making was interested in using it. So I noticed that the Popout module isn't handling foundry's V2 Applications.

image

I made some changes that allows my V2 application to render perfectly. And it doesn't seem to have any impact on V1 applications, since all I'm really doing is wrapping a JQuery context around some calls so that both the V1 and V2 apps can compute properly.

The only other thing I added was attaching a 'Popout' class to the main element. This is just something I think would be very usefull as it would allow modules to change their styling based on that class being present (something I needed).


Could very well be some important stuff I'm not considering since I'm not deeply familiar with the code, but give it a think 😁

@Posnet
Copy link
Contributor

Posnet commented Aug 29, 2024

Thanks for bringing ApplicationV2 support to my attention. However I'm unlikely to merge this request in the current state. I've read through the source for ApplicationV2, and since it is effectively a hard fork of the Application type, my preference is just to make corresponding popout2.js to handle the V2 apps independently. Though it is not actually quite that simple since they will need to share the window.ui proxy.

Separately, adding the popout class is not an option for the same reason I can't used the normal method for adding header buttons, there are event handlers that look for that class and take action that breaks popout out windows. Though this may have changed for ApplicationV2, and is another reason for having completely independent code paths for V1 and V2 applications.

@WBHarry
Copy link
Author

WBHarry commented Aug 29, 2024

Alrighty! Can close this PR then.
At the very least, the code as it stands is quite capable of handling v2 with some changes, that much I can attest 😁

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.

2 participants