Improve consistency of Item Sheet display (#4833)

less/v2/forms.less

@@ -168,6 +168,7 @@
     align-items: center;
     gap: 4px;
 
+    &:has(input[type=checkbox]:disabled, dnd5e-checkbox[disabled]) { cursor: default; }
     > span { line-height: calc(var(--form-field-height) + 1px); }
   }
 
@@ -408,7 +409,7 @@
 
   select {
     height: var(--form-field-height);
-    cursor: pointer;
+    &:not(:disabled) { cursor: pointer; }
   }
 
   :is(document-tags, multi-select) .tags.input-element-tags .tag {
@@ -420,6 +421,12 @@
     &:hover a { text-shadow: 0 0 6px var(--color-shadow-primary); }
   }
 
+  :is(document-tags, multi-select):has(select:disabled) .tags.input-element-tags .tag {
+    cursor: default;
+    a { cursor: default; }
+    &:hover a { text-shadow: none; }
+  }
+
   multi-select.hidden-tags .tags.input-element-tags { display: none; }
 
   :is(file-picker, document-tags) > button {
@@ -474,7 +481,7 @@
   dnd5e-checkbox {
     &[disabled]:not([checked]) {
       --checkbox-border-color: var(--color-text-light-6);
-      --checkbox-empty-color: var(--color-text-light-6);
+      --checkbox-empty-color: transparent;
     }
   }
 

module/applications/actor/sheet-v2-mixin.mjs

@@ -2,6 +2,7 @@ import * as Trait from "../../documents/actor/trait.mjs";
 import { formatDistance, formatNumber, simplifyBonus, splitSemicolons, staticID } from "../../utils.mjs";
 import Tabs5e from "../tabs.mjs";
 import DocumentSheetV2Mixin from "../mixins/sheet-v2-mixin.mjs";
+import ItemSheet5e2 from "../item/item-sheet-2.mjs";
 
 /**
  * Adds common V2 Actor sheet functionality.
@@ -599,8 +600,9 @@ export default function ActorSheetV2Mixin(Base) {
       const item = this.actor.items.get(itemId);
 
       switch ( action ) {
-        case "edit": item?.sheet.render(true); break;
         case "delete": item?.deleteDialog(); break;
+        case "edit": item?.sheet.render(true, { mode: ItemSheet5e2.MODES.EDIT }); break;
+        case "view": item?.sheet.render(true, { mode: ItemSheet5e2.MODES.PLAY }); break;
       }
     }
 

module/applications/components/checkbox.mjs

@@ -40,6 +40,8 @@ export default class CheckboxElement extends AdoptedStyleSheetMixin(
       aspect-ratio: 1;
     }
 
+    :host(:disabled) { cursor: default; }
+
     :host > div {
       width: 100%;
       height: 100%;

module/applications/components/inventory.mjs

@@ -2,6 +2,7 @@ import Item5e from "../../documents/item.mjs";
 import {parseInputDelta} from "../../utils.mjs";
 import CurrencyManager from "../currency-manager.mjs";
 import ContextMenu5e from "../context-menu.mjs";
+import ItemSheet5e2 from "../item/item-sheet-2.mjs";
 
 /**
  * Custom element that handles displaying actor & container inventories.
@@ -424,8 +425,9 @@ export default class InventoryElement extends HTMLElement {
       case "duplicate":
         return item.clone({name: game.i18n.format("DOCUMENT.CopyOf", {name: item.name})}, {save: true});
       case "edit":
+        return item.sheet.render(true, { mode: ItemSheet5e2.MODES.EDIT });
       case "view":
-        return item.sheet.render(true);
+        return item.sheet.render(true, { mode: ItemSheet5e2.MODES.PLAY });
       case "equip":
         return item.update({"system.equipped": !item.system.equipped});
       case "expand":

module/applications/item/item-compendium.mjs

@@ -1,4 +1,5 @@
 import Item5e from "../../documents/item.mjs";
+import ItemSheet5e2 from "./item-sheet-2.mjs";
 
 /**
  * Compendium with added support for item containers.
@@ -46,4 +47,13 @@ export default class ItemCompendium5e extends Compendium {
     // Let parent method perform sorting
     super._handleDroppedEntry(target, item.toDragData());
   }
+
+  /* -------------------------------------------- */
+
+  /** @override */
+  async _onClickEntryName(event) {
+    const { entryId } = event.target.closest("[data-entry-id]")?.dataset ?? {};
+    const item = await this.collection.getDocument?.(entryId);
+    item?.sheet.render(true, { mode: this.collection.locked ? ItemSheet5e2.MODES.PLAY : ItemSheet5e2.MODES.EDIT });
+  }
 }

module/applications/item/item-directory.mjs

@@ -1,4 +1,5 @@
 import Item5e from "../../documents/item.mjs";
+import ItemSheet5e2 from "./item-sheet-2.mjs";
 
 /**
  * Items sidebar with added support for item containers.
@@ -24,4 +25,13 @@ export default class ItemDirectory5e extends ItemDirectory {
     // Let parent method perform sorting
     super._handleDroppedEntry(target, item.toDragData());
   }
+
+  /* -------------------------------------------- */
+
+  /** @override */
+  async _onClickEntryName(event) {
+    const { entryId } = event.target.closest("[data-entry-id]")?.dataset ?? {};
+    const item = this.collection.get(entryId);
+    item?.sheet.render(true, { mode: ItemSheet5e2.MODES.EDIT });
+  }
 }

module/applications/item/item-sheet.mjs

@@ -262,17 +262,18 @@ export default class ItemSheet5e extends ItemSheet {
 
   /**
    * Get the base weapons and tools based on the selected type.
+   * @param {object} [context]        Sheet preparation context.
    * @returns {Promise<object|null>}  Object with base items for this type formatted for selectOptions.
    * @protected
    */
-  async _getItemBaseTypes() {
+  async _getItemBaseTypes(context) {
     const baseIds = this.item.type === "equipment" ? {
       ...CONFIG.DND5E.armorIds,
       ...CONFIG.DND5E.shieldIds
     } : CONFIG.DND5E[`${this.item.type}Ids`];
     if ( baseIds === undefined ) return null;
 
-    const baseType = this.item.system.type.value;
+    const baseType = context?.source.type.value ?? this.item.system.type.value;
 
     const items = {};
     for ( const [name, id] of Object.entries(baseIds) ) {

module/applications/item/sheet-v2-mixin.mjs

@@ -111,7 +111,6 @@ export default function ItemSheetV2Mixin(Base) {
         user: game.user,
 
         // Physical items
-        baseItems: await this._getItemBaseTypes(),
         isPhysical: "quantity" in this.item.system,
 
         // Identified state
@@ -134,6 +133,10 @@ export default function ItemSheetV2Mixin(Base) {
       context.inputs = { ...foundry.applications.fields, ...dnd5e.applications.fields };
       const { description, identified, schema, unidentified, validProperties } = this.item.system;
       context.fields = schema.fields;
+      if ( !context.editable ) context.source = context.system;
+
+      // Physical items
+      context.baseItems = await this._getItemBaseTypes(context);
 
       // Set some default collapsed states on first open.
       if ( foundry.utils.isEmpty(this._collapsed) ) Object.assign(this._collapsed, {
@@ -148,7 +151,7 @@ export default function ItemSheetV2Mixin(Base) {
       context.tabs = this.constructor.TABS.reduce((tabs, { tab, label, condition }) => {
         if ( !condition || condition(this.item) ) tabs.push({
           tab, label,
-          classes: ["item", activeTab === tab ? "active" : null].filterJoin(" ")
+          classes: ["item", "interface-only", activeTab === tab ? "active" : null].filterJoin(" ")
         });
         return tabs;
       }, []);
@@ -174,7 +177,11 @@ export default function ItemSheetV2Mixin(Base) {
         object: Object.fromEntries((context.system.properties ?? []).map(p => [p, true])),
         options: (validProperties ?? []).reduce((arr, k) => {
           const { label } = CONFIG.DND5E.itemProperties[k];
-          arr.push({ label, value: k, selected: this.item._source.system.properties?.includes(k) });
+          arr.push({
+            label,
+            value: k,
+            selected: context.source.properties?.includes?.(k) ?? context.source.properties?.has?.(k)
+          });
           return arr;
         }, [])
       };
@@ -242,6 +249,24 @@ export default function ItemSheetV2Mixin(Base) {
       // Play mode only.
       if ( this._mode === this.constructor.MODES.PLAY ) {
         html.find(".sheet-header .item-image").on("click", this._onShowIcon.bind(this));
+        this._disableFields(this.form);
+      }
+    }
+
+    /* -------------------------------------------- */
+
+    /**
+     * Disable form fields that aren't marked with the `interface-only` class.
+     * @param {HTMLElement} form  The form element whose fields are being disabled.
+     */
+    _disableFields(form) {
+      const selector = `:is(${[
+        "INPUT", "SELECT", "TEXTAREA", "BUTTON", "DND5E-CHECKBOX", "COLOR-PICKER", "DOCUMENT-TAGS",
+        "FILE-PICKER", "HUE-SLIDER", "MULTI-SELECT", "PROSE-MIRROR", "RANGE-PICKER", "STRING-TAGS"
+      ].join(", ")}):not(.interface-only)`;
+      for ( const element of form.querySelectorAll(selector) ) {
+        if ( element.tagName === "TEXTAREA" ) element.readOnly = true;
+        else element.disabled = true;
       }
     }
 

module/applications/mixins/sheet-v2-mixin.mjs

@@ -37,10 +37,10 @@ export default function DocumentSheetV2Mixin(Base) {
 
     /**
      * The mode the sheet is currently in.
-     * @type {ActorSheetV2.MODES}
+     * @type {ActorSheetV2.MODES|null}
      * @protected
      */
-    _mode = this.constructor.MODES.PLAY;
+    _mode = null;
 
     /* -------------------------------------------- */
 
@@ -51,6 +51,20 @@ export default function DocumentSheetV2Mixin(Base) {
     /*  Rendering                                   */
     /* -------------------------------------------- */
 
+    /** @inheritDoc */
+    async _render(force, { mode, ...options }={}) {
+      if ( !this._mode ) {
+        this._mode = mode ?? this.constructor.MODES.PLAY;
+        if ( this.rendered ) {
+          const toggle = this.element[0].querySelector(".window-header .mode-slider");
+          toggle.checked = this._mode === this.constructor.MODES.EDIT;
+        }
+      }
+      return super._render(force, options);
+    }
+
+    /* -------------------------------------------- */
+
     /** @inheritDoc */
     async _renderOuter() {
       const html = await super._renderOuter();

module/data/item/consumable.mjs

@@ -173,7 +173,10 @@ export default class ConsumableData extends ItemDataModel.mixin(
       ...this.physicalItemSheetFields
     ];
     context.damageTypes = Object.entries(CONFIG.DND5E.damageTypes).map(([value, { label }]) => {
-      return { value, label, selected: context.source.damage.base.types.includes(value) };
+      return {
+        value, label,
+        selected: context.source.damage.base.types.includes?.(value) ?? context.source.damage.base.types.has(value)
+      };
     });
     context.denominationOptions = [
       { value: "", label: "" },

module/data/item/weapon.mjs

@@ -267,7 +267,10 @@ export default class WeaponData extends ItemDataModel.mixin(
 
     // Damage
     context.damageTypes = Object.entries(CONFIG.DND5E.damageTypes).map(([value, { label }]) => {
-      return { value, label, selected: context.source.damage.base.types.includes(value) };
+      return {
+        value, label,
+        selected: context.source.damage.base.types.includes?.(value) ?? context.source.damage.base.types.has(value)
+      };
     });
     const makeDenominationOptions = placeholder => [
       { value: "", label: placeholder ? `d${placeholder}` : "" },

templates/actors/parts/actor-classes.hbs

@@ -4,11 +4,11 @@
     {{#dnd5e-itemContext cls as |ctx|}}
     <div class="class pill-lg" data-item-id="{{ cls.id }}" style="--underlay: url('{{ ctx.prefixedImage }}');">
         <div class="icons {{#if cls.subclass}}subclass{{/if}}">
-            <img class="gold-icon item-tooltip" src="{{ cls.img }}" alt="{{ cls.name }}" data-action="edit"
+            <img class="gold-icon item-tooltip" src="{{ cls.img }}" alt="{{ cls.name }}" data-action="view"
                  data-item-id="{{ cls.id }}">
             {{#if cls.subclass}}
             <img class="gold-icon item-tooltip" src="{{ cls.subclass.img }}" alt="{{ cls.subclass.name }}"
-                 data-action="edit" data-item-id="{{ cls.subclass.id }}">
+                 data-action="view" data-item-id="{{ cls.subclass.id }}">
             {{else if ctx.needsSubclass}}
             <a class="gold-icon subclass-icon" data-action="findItem" data-item-type="subclass"
                data-class-identifier="{{ cls.identifier }}" data-tooltip="DND5E.SubclassAdd"

templates/actors/tabs/character-details.hbs

@@ -127,7 +127,7 @@
                 </div>
                 {{/with}}
                 {{#if race}}
-                <div class="pill-lg texture race item-tooltip" data-action="edit"
+                <div class="pill-lg texture race item-tooltip" data-action="view"
                      data-item-id="{{ system.details.race.id }}"
                      aria-label="{{#if actor.isOwner}}{{ localize "DND5E.ItemEdit" }}{{else}}{{ localize "DND5E.ItemView" }}{{/if}}">
                     {{#if system.details.race.img}}
@@ -156,7 +156,7 @@
                 </div>
                 {{/if}}
                 {{#if background}}
-                <div class="pill-lg texture background item-tooltip" data-action="edit"
+                <div class="pill-lg texture background item-tooltip" data-action="view"
                      data-item-id="{{ system.details.background.id }}"
                      aria-label="{{#if actor.isOwner}}{{ localize "DND5E.ItemEdit" }}{{else}}{{ localize "DND5E.ItemView" }}{{/if}}">
                     {{#if system.details.background.img}}

templates/shared/inventory2.hbs

@@ -46,7 +46,7 @@
         <ul class="containers unlist">
             {{#each containers}}
             <li class="container" data-item-id="{{ id }}">
-                <a class="item-action" data-action="edit" data-tooltip="{{ name }}" aria-label="{{ name }}">
+                <a class="item-action" data-action="view" data-tooltip="{{ name }}" aria-label="{{ name }}">
                     {{#dnd5e-itemContext this as |ctx|}}
                     {{#with ctx.capacity}}
                     <div class="meter progress" role="meter" aria-valuemin="0" aria-valuenow="{{ pct }}"
@@ -147,7 +147,7 @@
 
                         {{!-- Item Name --}}
                         <div class="item-name item-action item-tooltip {{ @root.rollableClass }}" role="button"
-                             data-action="{{#if (eq item.type "container")}}edit{{else}}use{{/if}}"
+                             data-action="{{#if (eq item.type "container")}}view{{else}}use{{/if}}"
                              aria-label="{{ item.name }}">
                             {{#if ctx.concentration}}
                             <i class="concentration fas fa-arrow-rotate-left fa-spin fa-spin-reverse"