.header-button class missing from PopOut header button

[kristianserrano]

I noticed the .header-button class is missing from the PopOut button in the header. This prevents other modules from styling or otherwise modifying header buttons consistently. Is there a specific reason why it's not present, or is it just an oversight?

[admiralnlson]

My PR #65 from last week fixes this already. All that is needed now is a PopOut! release that includes it.

[Posnet]

After further testing I have identified that the header-button class is reserved by use in the core foundry module and have had to remove it. If you are manually adding that class name to application objects then you will be generating errors due to the fact that foundry expects all elements with that class to have a corresponding event handler registered with the application. See line 3202 of foundry.js for version 9.255. I consider any change that introduces unhandled errors to the core foundry application a bug. I am open to alternative suggestions on how to solve this.

[zeel01]

This is because as of a few versions ago, Foundry has standardized the method of adding buttons to application headers. The hook:

 Hooks.call(`get${cls.name}HeaderButtons`, this, buttons);

Called on line 3308 in the _getHeaderButtons method. All header buttons should be provided to the buttons object via this hook. Foundry will then appropriately display them, attach their event handlers, etc.

It appears that a much older method, likely pre-dating hooks, is still being used here involving jQuery. A more substantial refactor will be needed to upgrade the module to utilize the latest API. But the results will be far improved compatibility with other modules.

[Posnet]

Thanks for pointing out this new API. It was needed for compatibility with 0.7. However since I have dropped support for that version I'll consider switching. However there were ordering issues related to relying on hooks that I will need to confirm are no longer a problem.

[Posnet]

@zeel01 unfortunately that API actually won't work for the popout use case, since it requires knowing the class name of all windows that might be popped out. I also can't use the windows proxy hack I currently have in place to intercept the buttons array since by that point the window has already been rendered.

[zeel01]

You should only need to hook getApplicationHeaderButtons, the hook is fired for the entire app inheritance chain.

[Posnet]

@zeel01 I have done further testing, and it still does not work. "getApplicationHeaderButtons" is never called since _getInheritanceChain does to walk to an arbitrary base, it walks until it reaches the defined baseApplication property.