#Version 12 Deep Dive - Application V2

1 messages · Page 2 of 1

austere shadow
#

yup if you want to add them like the core buttons you have to manually insert them

#

(in which case they also won't go into the collapse menu when the window is small)

ancient crescent
#

Header buttons are something that should get added to the wiki pages

austere shadow
#

I'm still not a fan of the current solution; a dropdown should only be present IMO when needed, otherwise it's just a hassle for users.

So I'm personally just inserting my header buttons manually like the core sheets do, and making sure to set min-width as needed on the application.

ancient crescent
#

Yeah automatic width detection would be nice

wicked violet
#

So I'm personally just inserting my header buttons manually like the core sheets do

If everyone does this, we’re exactly right back where we started.

strong stream
wicked violet
#

Personally I think devs should follow the core intent here and let their icons be dumped in the submenu, and if there’s a demand for spilling them back out, write a module.

humble sorrel
#

In game engine development, it is always in your best interest to not fight core patterns

strong stream
#

Yeah, it's best not to fight core on principle. But if core deviates from what someone wants to achieve, it's gonna end up happening regardless. There's a balance to be had between core suggesting good practices and devs achieving what they want

wicked violet
#

What I’m saying is perhaps better put more precisely: “I have a module that adds an icon to window headers” and “I want my window header icons outside the submenu” are two separate ideas that shouldn’t be conflated in a single package.

humble sorrel
austere shadow
#

From my personal perspective, the thing about the header buttons is all to-do with accessibility.
If people don't know that a button exists, they won't click a dropdown-menu to go looking for it.

If I take the 'Prototype Token' button from v1 ActorSheets as an example:

If that was in a dropdown, how many people would be reporting that they can't find a way to edit their prototype tokens?

You could write a guide and tell people the button is there, which'll stop a decent chunk of questions, but there'll still be those that didn't read the guide and ask the question, at which point you'll have to point them to the guide.

Or... alternatively... the button is obvious and visible right away from the start.

wicked violet
#

I do agree that there’s a discoverability issue here, that would probably be best solved by auto-collapsing overflowing icons into the submenu. But in the meantime, I think it’s not the right approach to force-place them into the header (unless doing so as a discrete module for this purpose).

austere shadow
#

I'd argue otherwise, I'd rather have the functionality I desire for the sheets I create and solve the problem of "the close button disappears if the window is too small" myself, then deal with a degraded user experience.

#

If this was purely a point of API Verbosity where I have to write more code to do the same thing, I'd agree with you, but since this will degrade usability of your applications for your users, I can't agree with your point.

wicked violet
#

It’s your call of course, but if the standard expectation is that module-added icons drop into the submenu, and I install one that forces it outside, that module is breaking my expectations of how the software works.

And if someone else comes along and publishes a module that implements auto-collapse, spilling out all submenu icons until the window gets too small or whatever, then all the core-pattern modules “just work” with this, while the ones that force placement now appear broken.

ancient crescent
#

QUESTION FOR STAFF: I see that the type ApplicationHeaderControlsEntry provides a visible boolean, however

  • Why is this a boolean rather than a callback that returns a boolean?
  • I don't see where it's ever actually used in the appV2 class
steel dome
#

Anyway, header buttons aside. I rewrote my item sheet to AppV2 today, and it turned out to be trivial, besides the lack of drag-drop functionality. So that was nice.

kindred minnow
#

I'd like to throw out there, that having the header buttons in a menu on the window isn't all that hidden (Honestly compared to many right click options, such as scenes navigation and players) the header menu in AppV2 is much more obvious from a UX perspective.

As an End user, I really hate header buttons, unless they have a 100% justifiable reason. For example, window controls and taskbar modules that add, min and max Buttons, totally makes sense for them to be main buttons. 99% of other buttons could easily fall into a menu in my personal opinion.

Unless your button is something that should be interacted with more than 50% of the time when the application is open, it being under a menu makes sense to me... At least that's my opinion.

#

Basically, I think the default, should be within the menu as core intends, unless your module makes a very good use case for it being a main button.

humble sorrel
#

Header buttons are often cop outs

kindred minnow
# humble sorrel Header buttons are often cop outs

That's not really true. They totally make sense for things like window controls module. I agree to some extent, but I don't think they are a completely cop out. Don't you have a module with a header button?

steel dome
#

Yes badger, how does one revert a warpgate mutation. zhellsmug

humble sorrel
#

Since then, I have done everything I can to not use header buttons

strong stream
#

I think both stances are true to a degree. There are times when they're used unnecessarily, but there are times when they're appropriate. Something system-agnostic that's directly tied to a document often doesn't have a better spot to go

ancient crescent
#

SWADE has two instances of header buttons;

  1. "Link", which we probably don't need as much now, but it copies the link to clipboard
  2. "Tweaks", which is our bonus menu of extra configuration; dnd5e has a similar menu but it's in the sheet body
#

both work just fine in the dropdown menu

kindred minnow
#

Which brings me back to my major point... If your module can be like... It needs this header button because it's what the module does (window controls / taskbar / window tabs) but otherwise you can probably find a better way aka not finding a better way is kinda a cop out.

steel dome
#

Speaking of header buttons ( 🙂 )
Hitting Enter while in an input field seems to trigger whatever button is first in that collapsed container. Don't suppose anyone could help confirm that, so I know it's not just my jank item sheet?

humble sorrel
#

That is expected html form submission default behavior

#

You can define an inert submit. Or.... Something else that is not coming to mind

steel dome
#

OK thank you, wasnt aware header buttons were meant to be submit buttons

humble sorrel
#

I may have misread...but it sounds like a similar thing 🤔

ancient crescent
#

Header buttons shouldn't be triggering submit when you're focusing that submenu

steel dome
#

Not focused on it at all. Are you seeing the same? It's just from hitting Enter in any input field in the form itself.

ancient crescent
#

I haven't been doing work

karmic current
vivid beacon
#

(Month old reply, but figured it's good for context - especially since people seem to find it working well 😁 )

So, I went ahead with Athro's Crucible solution to Tabs to solve the initial-tab problem, but it's not really making sense to me.
Sure, I'm rendering the Nav with it's different tabs and setting an initial active class which will work - but this doesn't have any effect on the actual content of the tab. It'll still be set as display: none. If I set some class on it explicitly according to tabs values, then it seems to me like I'll just be overwriting/fighting the existing handler for changing tabs in ApplicationV2. Guess I'm missing something in this solution 🤔

eager hill
#

For example in one of the tab templates, like crucible/templates/sheets/partials/item-actions.hbs the containing element of the tab is declared like:

<section class="tab sheet-body flexcol {{tabs.actions.cssClass}}"
         data-tab="{{tabs.actions.id}}" data-group="{{tabs.actions.group}}">
#

tabs.actions.cssClass sets the initial "active" class (if applicable) to the tab when it is initially rendered (or re-rendered).

vivid beacon
steel dome
#

AppV2 should be setting some Sheet#tabGroup value you can read from

eager hill
vivid beacon
# eager hill No, the application class populates `this.tabGroups` with the correct active val...

It does. I guess I'll just have to have some logic in prepareContext to select tabGroup values instead of _getTabs values if they are available.
I was working off the logic of keeping a separate data-value store for the initial-tab problem.

  _getTabs() {
      return {
          features: { active: true, cssClass: 'active', group: 'primary', id: 'features', icon: null, label: 'Features' },
          effects: { active: false, cssClass: '', group: 'primary', id: 'effects', icon: null, label: 'Effects' },
      }
  }
#

Oh. Now I realised the second part of what you were doing to handle that neatly. Nvm. Good solution 😅

eager hill
#

yeah, you shouldn't need any secondary data store

#

just pass your tabs into the templates

steel dome
#

Additional header buttons are not localized automatically in _renderHeaderControl. Should they not be, or is the intent that users override _getHeaderControls?

ancient crescent
#

I hadn't checked the time it's called; can you just run it through game.i18n.localize?

#

but it would make sense that it should be auto-localized

steel dome
ancient crescent
#

Ah yeah that's not great

eager hill
#

I think that they should be automatically localized

#

that would be worth a github issue so I can track it

steel dome
#

done

steel dome
#

I sometimes have a use case for assigning an additional css class to an input field. I see this as an option when using {{formField}} but not {{formInput}}, is that right?

ancient crescent
#

What's the best practice for conditionally showing a part, e.g. only showing the bio tab on a document sheet if document ownership is Limited?

ancient crescent
#

as that's where options.parts gets assigned

eager hill
ancient crescent
eager hill
ancient crescent
#

Ah perfect thank you

ancient crescent
#

@eager hill question about ActorSheetV2 — do you plan to incorporate DragDrop? Or should Boilerplate do it?

#

I'm working on the v12 branch for Boilerplate and trying to suss out how much systems are responsible for

eager hill
ancient crescent
#

Okay, I'll poke around to see how building it in might work

eager hill
#

It would just look something like this:

_onRender(context, options) {
  const dd = new DragDrop({dragSelector: ".drag-this-thing", dropSelector: ".drop-onto-here"});
  dd.bind(this.element);
}
#

You can, of course, pass permissions and callbacks into the DragDrop constructor to fine-tune behavior.

ancient crescent
#

Yeah it's definitely doable just a question of how to make it accessible to someone who's building off of Boilerplate

ancient crescent
#

btw I gotta say I really like appV2 overall, there's a learning curve but the end result is better

ancient crescent
steel dome
#

In my case I just took a leaf from dnd5e and had "Edit" and "Play" mode for actor and item sheets. Edit mode always shows the source data, Play mode shows data of the document as it is with AEs and derived data and with all fields disabled.

ancient crescent
#

also an option. Are you literally calling this.actor.source?

steel dome
#

toObject()

ancient crescent
#

ah yeah so functionally

steel dome
#

With {{formField}} it just results in a single ternary operator to get the correct value, either from the document or from the source data.

ancient crescent
#

that's pretty slick

steel dome
#

Preparing every field with a quick function like so.

    const context = { ... };

    const makeField = (path, formula = true) => {
      const field = doc.system.schema.getField(path);
      const dv = foundry.utils.getProperty(doc.system, path);
      const src = foundry.utils.getProperty(context.source, path);
      let value;

      if (formula) {
        if (!dv || (dv === "0")) value = "";
        else if (context.isPlayMode) value = artichron.utils.simplifyBonus(dv, rollData);
        else value = src;
      } else {
        value = context.isPlayMode ? dv : src;
      }

      return {field: field, value: value, disabled: context.isPlayMode};
    };
#

I did make use of overrides at first, but this ended up being much less effort since overrides doesn't easily cover every use case (e.g., arrays).

austere shadow
#

so having moved over to {{formField}} for everything in my apps where possible, I've gotten perhaps a slightly nonsensical request, so I'd love to hear both the plausibility and also how stupid of an idea this would be 😛

Would it be possible and sensible to have 'Calculated only' DataFields? So I can still setup their rendering using the data field type, labels & hints, but have the field be ignored during .updates and not persist to the DB?

I know I could just make the calculated fields into data fields and still edit them directly as needed, but then the data will persist into the DB on updates which seems like unnecessary overhead?

I have a feeling I may be missing something that makes this undesirable behavior, but from a consistency perspective, it'd be great if I could render calculated fields like that with formField and formInput, instead of having to manually write out the HTML because the field doesn't exist.

Thoughts?

steel dome
#

Make a field in memory?

austere shadow
#

I... suppose that would work now wouldn't it.

steel dome
#

Don't think anything prevents you from doing this:

      const field = new foundry.data.fields.StringField({
        choices: choices,
        label: "MyLabel",
        initial: ""
      });
#

I do this in a Dialog for the lulz

austere shadow
#

Hah.

jovial hatch
#

Creating fields like that for context preparation is easy enough. Another context in which schema fields for derived (and not persisted) data could be interesting though would be Active Effect application.

vivid beacon
#

Few things relating to the {{formField ...}} helper and V2 context prep in general as I'm trying it out:
I tossed in useage of the helper right next to the old implementation to compare.

{{formField fields.system.actionType value=source.system.actionType rootId=partId label="DAGGERHEART.Sheets.Feature.FeatureType" localize=true }}
<div class="form-group">
    <label>{{localize "DAGGERHEART.Sheets.Feature.ActionType"}}</label>
    <div class="form-fields">
         <select name="system.actionType">
            {{selectOptions this.itemConfig.actionTypes selected=document.system.actionType labelAttr="name" localize=true blank="" }}
        </select>
    </div>
</div>

The data model for the actionType field is: actionType: new fields.StringField({ choices: Object.keys(SYSTEM.ITEM.actionTypes), initial: SYSTEM.ITEM.actionTypes.passive.id }),
How would I go about altering things to have value/name setup in the select using the helper? 🤔

steel dome
#

You can drop the Object.keys

vivid beacon
#

Dropping it was fine, so a bit less clutter. It doesn't do anything for the select setup though. It's still just using the key as value and name. In the old way this was solved by labelAttr to use the name field

steel dome
#

Can you not still pass that?

vivid beacon
#

It's not being read, no.

#

Another thing I'm pondering is how one would utilize schema.fields well in a DocumentSheet data structure, IE the data structure is not flat and system exists. I wanted to set up a Sheet class with some general preparation as I've seen a lot of around here:

async _prepareContext(_options) {
      const context = await super._prepareContext(_options);
      context.source = this.document.toObject();
      context.fields = this.document.schema.fields;

      return context;
    }

From my attempt, context.fields doesn't get any field data for system and beyond here.
I can of course set up context.fields.system = this.document.system.schema.fields;, but I'm assuming it would become a recurring issue as there might be EmbeddedFields in there and so on.
Curious if anyone knows a good way to make this work neatly, as I'd like easy useage for formField helpers in my html: {{formField fields.system.<MyField> value=source.system.<MyField> }}

steel dome
#

Does look like labelAttr is being used, but maybe dropped somewhere down the line.

vivid beacon
steel dome
#

Looks like it does not get passed to StringField._getChoices

ancient crescent
#

Schema traversal breaks at embedded data models

steel dome
#

So I thought this would work, but it does not.

{{formField myField value=value localize=true labelAttr="steve"}}

where

myProperty: new NumberField({
  choices: {
    1: {steve: "UnlocalizedOption1"},
    2: {steve: "UnlocalizedOption2"}
  },
  initial: 1,
})
austere shadow
steel dome
#

That works. It also works if labelAttr="label"

#

But not if it is anything else.

vivid beacon
#

But changing things to label could be doable.

vivid beacon
austere shadow
# austere shadow what I do is set my choices as followed: ```js new fields.stringField({ choice...

For "basic" properties we are able to leverage the LOCALIZATION_PREFIXES and then it also works iirc if you f.e. have actor.system.someField set to the above schema I'm quoting.

Set the localization prefix to f.e. ["SYSTEM.ActorSystem"]

and then in your localization file have

{ 
  "SYSTEM": {
    "ActorSystem": {
      "FIELDS": {
        "someField": {
          "label": "Label for this field",
          "hint": "Hint text for this field",
          "option": "localized option 1",
          "option2": "localized option 2"
        }
      }
    }   
  }
}
#

however this fully falls appart if you are trying to use fields that are embedded in other fields (such as ArrayField(SchemaField))

#

(unless this has been patched since Dev 2, but I believe it hasn't)

ancient crescent
vivid beacon
ancient crescent
austere shadow
# vivid beacon Hm. So more or less, it's not really feasible to have a generic Sheet class that...

The way I've personally set this up, is I've created MySystemItemSheetV2
with shared parts for the layout and then change the template of the 'details' tab of my sheet (where all the fields would go)

Then if I want to make say a 'weapon sheet' I extend MySystemItemSheetV2 and do:

static PARTS = foundry.utils.mergeObject(super.PARTS, {
  details: {
    template: "/path/to/weapon-sheet/specific/handlebars/file"
  }
}, { inplace: false });

which then in turns looks something like this:

<section class="tab sheet-body {{tabs.details.cssClass}}" data-tab="details" data-group="sheet">
  <fieldset>
      <legend>
          Description
      </legend>
      {{formField fields.description value=source.system.description toggled=true localize=true}}
  </fieldset>
</section>

(Ignore the fact I haven't updated for new HTMLField handling, haven't had the chance yet 😛)

ancient crescent
austere shadow
#

...Neat!

ancient crescent
#

Wait sorry PARTS not DEFAULT_OPTIONS

austere shadow
#

Yeah don't think PARTS is getting auto merged.

steel dome
#

Doesnt this modify the super class since mergeObject is inplace by default

austere shadow
#

It does, I have an inplace false, forgot to copy it over since I manually typed it out x)

#

edited in above x)

austere shadow
vivid beacon
#

Don't know if this is a bug, or if I should go about it a different way in V2. I checked, and it does work in V1. 🫠
Test case with the simplest possible data models.

export default class DhpTest extends foundry.abstract.TypeDataModel {
    static defineSchema() {
      const fields = foundry.data.fields;
      return {
        actions: new fields.ArrayField(new fields.EmbeddedDataField(DaggerheartAction)),
      }
    }
}

export default class DaggerheartAction extends foundry.abstract.DataModel {
    static defineSchema() {
      const fields = foundry.data.fields;
      return {
        name: new fields.StringField({ initial: 'New Action' }),
      }
    }
}

Doing any kind of update on the system.actions field, even an empty array, fails with an error, as the field property in SchemaField._cleanType becomes an object instead of a proper field, and therefore has no .clean function. IE, update below with error

static async addThing(){
    await this.document.update({ "system.actions": [] });
}

Edit: Adding a bug report on it

steel dome
#

Is it recommended to not use a subclass of DocumentSheetV2 for config menus? I am getting some strange errors when such a config is submitted while an actor sheet is rendered when the config is re-rendered while the actor sheet is also rendered. Weirdly claiming that all the entries in the actor sheet's PARTS are not supported template parts of the config menu, but the two are entirely unrelated beyond being tied to the same actor.

E: is bug. #v12-feedback message

austere shadow
#

I feel like I saw someone else already report this earlier but the AppV2 header dropdown should probably have a non-transparent background.

steel dome
#

... so this is how I learn that if you add your own buttons, you lose all the core ones.

#

Looks like DEFAULT_OPTIONS.window.controls is not concatenated.

austere shadow
#

@eager hill RE: https://github.com/foundryvtt/foundryvtt/issues/10897#issuecomment-2107705489

DocumentSheetV2 mimics the behavior of the V1 equivalent by assigning itself in document.apps. I have some apps that are openable from an ActorSheet that should be rerendered whenever the actor is updated, so I have those apps assign themselves to document.apps as well whenever they get opened, to make use of the built-in call to ClientDocumentMixin#render which is called whenever the document is updated. (See attached image)

/***
 * Render all of the Application instances which are connected to this document by calling their respective
 * @see Application#render
 * @param {boolean} [force=false]     Force rendering
 * @param {object} [context={}]       Optional context
 * @memberof ClientDocumentMixin#
 */
render(force=false, context={}) {
    for ( let app of Object.values(this.apps) ) {
      app.render(force, context);
    }
}

As you can see the context here is getting re-used, because of this what'll happen is the Actor Sheet has a certain amount of parts, and will assign that to the context, then when in this case the ComponentPopout render gets called the context already contains the parts from the ActorSheet, causing the ComponentPopout's render to complain that all those parts don't exist.

eager hill
#

I see. I think it's probably the responsibility of Document#render to pass a cloned context to each app than it is the responsibility of AppV2 to make a (usually unnecessary) copy.

#

I can approach it that way.

austere shadow
#

I'd imagine a simple change to app.render(force, foundry.utils.duplicate(context)) would resolve it yeah.

#

(or whichever more elegant solution you come up with 😛)

solar stone
#

For those of you with AppV2 sheets up and running already, how do you have your CSS (optionally Less/SCSS) set up? I'm curious as to the best way to style AppV2 sheets with as little breakage to V1 sheets

steel dome
#

appv2 has .application.
appv1 has .app

austere shadow
#

^

solar stone
#

Does AppV2 have things like window-content and sheet-header still?

steel dome
#

window-header and window-content, yeah

steel dome
#

So how does a module actually add a header button in the collapsed menu. I am going by the assumption it is not done by injecting an html element, though I am not seeing an alternative.

humble sorrel
#

(maybe naive answer) getHeaderButtons?

steel dome
#

Sure, it would have been my second guess that we'd override one of the prototype methods.

#

There isnt a hook if thats what you meant

humble sorrel
#

it was not, sorry, the parent class protected method

ancient crescent
#

ActorSheetV2 provides a decent demo of this


    window: {
      controls: [
        {
          action: "configurePrototypeToken",
          icon: "fa-solid fa-user-circle",
          label: "TOKEN.TitlePrototype",
          ownership: "OWNER"
        },
        {
          action: "showPortraitArtwork",
          icon: "fa-solid fa-image",
          label: "SIDEBAR.CharArt",
          ownership: "OWNER"
        },
        {
          action: "showTokenArtwork",
          icon: "fa-solid fa-image",
          label: "SIDEBAR.TokenArt",
          ownership: "OWNER"
        }
      ]
    },
steel dome
#

I had dismissed that a few updates back, but why not, let's try again

ancient crescent
#

this is something I had worked out while updating the appV2 page

steel dome
#

I knew about it, but had assumed it was still broken

#

Nope, still borked.

ancient crescent
#

what do you have?

steel dome
#

Well if you add to this, it just overrides the array entirely still. It is not concatenated.

ancient crescent
#

do you have a ticket for that already?

steel dome
#

Dont recall. Dont think so.

mild sierra
#

Trying to figure out if I am doing this in a way that makes sense... I want to add a tab to AmbientLightConfig in v12. The template is easy: I can add it to the AmbientLightConfig.PARTS object.

But now I have a problem---getTabs is private:

    #getTabs() {
      const tabs = {
        basic: {id: "basic", group: "sheet", icon: "fa-solid fa-lightbulb", label: "AMBIENT_LIGHT.SECTIONS.BASIC"},
        animation: {id: "animation", group: "sheet", icon: "fa-solid fa-play", label: "AMBIENT_LIGHT.SECTIONS.ANIMATION"},
        advanced: {id: "advanced", group: "sheet", icon: "fa-solid fa-cogs", label: "AMBIENT_LIGHT.SECTIONS.ADVANCED"}
      };
      for ( const v of Object.values(tabs) ) {
        v.active = this.tabGroups[v.group] === v.id;
        v.cssClass = v.active ? "active" : "";
      }
      return tabs;
    }

So instead I wrap _prepareContext, and when that returns the context object, I add my new tab to context.tabs at that point. But now I need to call the for loop from #getTabs again, so after adding the new tab,to the context object I call:

      for ( const v of Object.values(tabs) ) {
        v.active = this.tabGroups[v.group] === v.id;
        v.cssClass = v.active ? "active" : "";
      }

And finally, the footer tab is in the wrong order because I added my tab after that one. So I wrap _configureRenderOptions to fix the order:

function _configureRenderOptions(wrapped, options) {
  wrapped(options);
  const footer = options.parts.findSplice(elem => elem === "footer");
  if ( footer ) options.parts.push("footer");
}

This appears to work, but seems like a lot of steps just to get another tab, so I am wondering if maybe I am missing a simpler approach?

ancient crescent
#

I think Foundry is right to use private props, but sometimes it's a bit overzealous

mild sierra
# ancient crescent I think the request here is make getTabs public

Actually, I think there is a simpler solution: That tabs object should be static, as in AmbientLightConfig.TABS = { basic, ... }. Then #getTabs can remain private, and would look something like:

  const tabs = this.constructor.TABS;
  for ( const v of Object.values(tabs) ) {
        v.active = this.tabGroups[v.group] === v.id;
        v.cssClass = v.active ? "active" : "";
      }
  return tabs;

Much easier to add a tab, and if it were a static object, it would be feasible to reorder it. Although if you are depending on tab order for things, the tab object should really be an array. Javascript does not guarantee order of properties in objects.

Addendum: Added a git issue to request this change:
https://github.com/foundryvtt/foundryvtt/issues/11023

vivid beacon
#

Hm. Used the formField helper on a HTML field, and it does spit out an editor.
Only issue is that the HTML is only displayed while editing. After saving, only the toggle button to open it is shown without any visible text. Some property that needs to be added? 🤔
{{formField systemFields.description value=source.system.description localize=true toggled=true}}

tender horizon
vivid beacon
#

No, that snag has been navigated. It's got a height, and the value in the editor is saved. The HTMl value is just not being shown when not in edit mode.

steel dome
#

enriched=someEnrichedText