[#79] Hide name/desc for items not identified #1696

ccjmk · 2022-08-11T13:56:52Z

Resolves #79

Used system.identified to toggle between description.value and description.unidentified and between showing true name or a masked 'Unidentified XYZ' faked name in item sheet (weapon/equipment/consumable) and actor inventory.

Possible further steps:

Leveraging baseItem on weapons and equipments to improve masked name, for example turning "Unidentified Martial Meele" into "Unidentified Rapier" or "Unidentified Medium Armor" to "Unidentified Breastplate".
Showing a fake icon if not identified; either mystery-man or saving a different icon akin of the description treatment.
Hiding possible spoiler data, ranging from values on the Description tab like Price and Properties, to hiding the Effects tab and everythin on the details tab save for the basic equipment details. Maybe even just showing the Identified flag on the Details tab at all.
Following a suggestion by Arcanist in the League's discord, a context menu could be added for the GM to identify/unidentify items on the sidebar (similar to Forien's mystify mechanics)
Some way of showing on the sidebar if an item is identified or not, like a little FA icon next to the name or showing the masked name with some style, and revealing the true name on hover for the GM, or the likes.

module/utils.mjs

aaclayton · 2022-08-27T14:42:00Z

I presume gamemasters need to edit both the identified and unidentified item descriptions. Can we provide a better UX for that than needing to toggle the identified state in order to have access to the relevant text field?

ccjmk · 2022-09-01T11:35:58Z

@Fyorl question, I was still working on aaclayton latest comment (maybe I should have replied to that before?) but I imagine you closed this 'cause the v10-dev branch is closed, can't I just rebase this on whatever the new branch is ? Is the intention to move away from what I've been working on onto a different solution (in system or in core, as requested on the issue linked by andrew -- though my understanding of his reply was that this was The solution for now--to have systems implement their own solutions)

Fyorl · 2022-09-01T11:39:10Z

The v10-dev branch has become 2.0.x. I was not expecting GitHub to close all the PRs associated with v10-dev, sorry. I will go through and re-open those now. Thanks for pointing it out.

Fyorl · 2022-09-01T11:52:55Z

I seem to be lacking some permissions to change the target of these PRs and reopen them. Are you able to edit it and change the target to 2.0.x?

ccjmk · 2022-09-01T11:57:44Z

I don't seem to be able to change it here, but on a repo I have full ownership I can change the target branch from the Edit button that lets you change the PR title. Have you tried that one ? It should turn the target label into a selectable. Questionable UX xD (EDIT: for me, it just lets me change the title here, not the target branch)

ccjmk · 2022-09-24T18:00:19Z

Following @aaclayton's advice, this is my current iteration, but I'm not sold on the UI with both descriptions together. Also, I see a divide between how only the "Unidentified {type}" is shown as name and showing both descriptions.

that is for GMs; regular users just see one description:

Alternatively, something like this could be done for the name:

There's also the fact that the title of the item sheet still shows the full name, but I am unable to change it somehow. I tried changing the title inside item-sheet.mjs::defaultOptions() but it didn't work. Where would be the appropriate place for that ?

kaelad02 · 2022-09-25T11:10:55Z

I get what you mean about the appearance of having both descriptions showing at the same time. I wonder if changing the styling of the <h3> dividers to look like those on the details tab would help. At least it would look consistent and maybe that'll be enough to make it look better.

And just to make sure, these kind of dividers are what I'm referring to:

kaelad02 · 2022-09-25T11:17:06Z

As for the title problem, the DocumentSheet class does have a title property that seems to pull the document's name. Perhaps that's overriding whatever you're trying to change with defaultOptions. I'd try overriding title in ItemSheet5e.

krbz999 · 2022-09-25T12:26:42Z

Having two text editors in the same place seems like the way to go for ease of use.

However, the front page of the item sheet is notoriously annoying to work with, what with its height always resetting to less than the minimum with every change (or when dragging the sheet). If the inputs are stacked on each other vertically, the minimum height of the sheet needs an adjustment more than ever (or at minimum needs to stop resetting its height when dragged or when changing tabs).

Bit of a tangent, but I agree with Kaelad's idea.

ccjmk · 2022-09-27T17:32:05Z

Updated as per suggestions, plus I found a couple extra places where the name and/or description are shown that were not initially mentioned; PR should be reviewable now!

Description of changes so far:

Inventory list now shows the correct description when expanded (screenshot is from a GM, player does not see true name)

Image had the item name as hover text, used the masked name now.

Also, GM sees both descriptions, with the unidentified one boxed and labeled for better visibility:

player view:

arbron

The item descriptions are currently unusable (seen on Firefox):

arbron · 2022-10-13T18:40:19Z

module/utils.mjs

+ * @returns {string}      The localized name to show for the unidentified item.
+ * @private
+ */
+ export function unidentifiedName(item) {


None of these are properly localized:

Oops, I thought they were in the system! Thanks for the review, I will look into it asap (I am moving this weekend though so it might take some time to catch up)

module/applications/item/item-sheet.mjs

arbron · 2022-10-13T18:49:23Z

templates/items/consumable.hbs

                <input name="name" type="text" value="{{item.name}}" placeholder="{{ localize 'DND5E.ItemName' }}"/>
+                {{else}}
+                <input disabled name="ignored" type="text" value="{{ localize (dnd5e-unidentifiedName item) }}"/>


Disabled inputs are automatically ignored by form processing, so you don't need to worry about the name here.

Suggested change

<input disabled name="ignored" type="text" value="{{ localize (dnd5e-unidentifiedName item) }}"/>

<input type="text" value="{{ localize (dnd5e-unidentifiedName item) }}" disabled>

arbron · 2022-10-13T18:49:36Z

templates/items/equipment.hbs

                <input name="name" type="text" value="{{item.name}}" placeholder="{{ localize 'DND5E.ItemName' }}"/>
+                {{else}}
+                <input disabled name="ignored" type="text" value="{{ localize (dnd5e-unidentifiedName item) }}"/>


Suggested change

<input disabled name="ignored" type="text" value="{{ localize (dnd5e-unidentifiedName item) }}"/>

<input type="text" value="{{ localize (dnd5e-unidentifiedName item) }}" disabled>

arbron · 2022-10-13T18:52:28Z

templates/items/parts/item-description.hbs

+        {{#unless isIdentifiable}}
+            {{editor descriptionHTML target="system.description.value"
+                button=true editable=editable engine="prosemirror" collaborate=false}}
+        {{/unless}}
+        {{#if showIdentifiedDesc}}
+            {{editor descriptionHTML target="system.description.value"
+                button=true editable=editable engine="prosemirror" collaborate=false}}
+        {{/if}}


These can just be merged into a single editor instance:

Suggested change

{{#unless isIdentifiable}}

{{editor descriptionHTML target="system.description.value"

button=true editable=editable engine="prosemirror" collaborate=false}}

{{/unless}}

{{#if showIdentifiedDesc}}

{{editor descriptionHTML target="system.description.value"

button=true editable=editable engine="prosemirror" collaborate=false}}

{{/if}}

{{#if (or showIdentifiedDesc (not isIdentifiable)}}

{{editor descriptionHTML target="system.description.value"

button=true editable=editable engine="prosemirror" collaborate=false}}

{{/if}}

arbron · 2022-10-13T18:59:37Z

Adding align-content: stretch; to .dnd5e.sheet.item .sheet-body .tab seems to help with the descriptions being too small, but it will need to be tested to make sure it doesn't break stuff elsewhere.

arbron · 2022-10-13T19:00:22Z

templates/items/parts/item-description.hbs

+        {{/if}}
+        {{#if showUnidentifiedDesc}}
+            {{#if isGM}}
+                <hr class="flex0">


Is this <hr> just for spacing? It doesn't seem to show up visually. If so, it would probably be better to do with with CSS on the unidentified description box.

I think this <hr> was a remnent from showing a line that I removed, it will be gone.

arbron · 2022-10-13T19:01:03Z

templates/items/parts/item-description.hbs

+            {{#if isGM}}
+                <hr class="flex0">
+                <div class="unidentified-description">
+                <h3 class="flex0" >{{localize "DND5E.UnidentifiedDesc"}}</h3>


.flex0 doesn't seem to be a class in the CSS, is this doing anything?

arbron · 2022-10-13T19:01:42Z

templates/items/weapon.hbs

                <input name="name" type="text" value="{{item.name}}" placeholder="{{ localize 'DND5E.ItemName' }}"/>
+                {{else}}
+                <input disabled name="ignored" type="text" value="{{ localize (dnd5e-unidentifiedName item) }}"/>


Suggested change

<input disabled name="ignored" type="text" value="{{ localize (dnd5e-unidentifiedName item) }}"/>

<input type="text" value="{{ localize (dnd5e-unidentifiedName item) }}" disabled>

Open delete dialogue to confirm deletion of active effects on the effects page.

…fect.mjs Require confirmation to delete effects.

…-effect.mjs Revert "Require confirmation to delete effects."

arbron · 2023-10-31T20:59:44Z

@ccjmk Support for editing multiple items has been merged into the 2.4 branch. If you wanted to update this using those updates then we can take a look at this for either 2.4 or the next version :)

ccjmk · 2023-11-02T09:09:17Z

@arbron I sure would love to! I need to get my dev env up to speed probably because it's been like a year since I touched anything foundry-related, but I'll definitely want to move this forward. I'll hit you on discord if that's ok once I have my local branch updated and conflicts fixed.

…s description.unidentified instead; GM sees true name and both descriptions

arbron self-requested a review August 11, 2022 16:24

arbron added ui User interface related features or bugs feature request labels Aug 11, 2022

arbron assigned ccjmk Aug 11, 2022

arbron reviewed Aug 12, 2022

View reviewed changes

module/utils.mjs Outdated Show resolved Hide resolved

aaclayton mentioned this pull request Aug 29, 2022

Support GM Only description or notes fields attached to Actors and Items foundryvtt/foundryvtt#836

Open

Fyorl deleted the branch foundryvtt:2.4.x September 1, 2022 11:23

Fyorl closed this Sep 1, 2022

Fyorl reopened this Sep 1, 2022

Fyorl changed the base branch from v10-dev to 2.0.x September 1, 2022 13:08

ccjmk force-pushed the feature/79-hide-unidentified-item-details branch from f9ce0c9 to 1b01c79 Compare September 1, 2022 15:59

ccjmk marked this pull request as draft September 1, 2022 17:28

ccjmk force-pushed the feature/79-hide-unidentified-item-details branch 4 times, most recently from f209aaa to 087575b Compare September 24, 2022 17:33

ccjmk marked this pull request as ready for review September 24, 2022 18:00

ccjmk force-pushed the feature/79-hide-unidentified-item-details branch from 087575b to b2785ae Compare September 27, 2022 17:24

ccjmk changed the base branch from 2.0.x to 2.1.x September 27, 2022 17:34

ccjmk requested a review from arbron October 12, 2022 17:26

arbron requested changes Oct 13, 2022

View reviewed changes

arbron reviewed Oct 13, 2022

View reviewed changes

module/applications/item/item-sheet.mjs Show resolved Hide resolved

arbron reviewed Oct 13, 2022

View reviewed changes

ccjmk mentioned this pull request Feb 4, 2023

[#2135, #2136] Add multi editor to make additional descriptions editable #2137

Closed

Fridan99 mentioned this pull request Feb 28, 2023

[Feature Request] Mistify items #2226

Closed

etiquettestartshere and others added 4 commits October 30, 2023 12:55

Require confirmation to delete effects.

5b1bf78

Open delete dialogue to confirm deletion of active effects on the effects page.

Merge pull request foundryvtt#2537 from etiquettestartshere/active-ef…

8be08e9

…fect.mjs Require confirmation to delete effects.

Revert "Require confirmation to delete effects."

bff512b

Merge pull request foundryvtt#2539 from foundryvtt/revert-2537-active…

a7f1404

…-effect.mjs Revert "Require confirmation to delete effects."

ccjmk added 5 commits November 5, 2023 19:09

Hides name and description for players if item is unidentified - show…

1fc3004

…s description.unidentified instead; GM sees true name and both descriptions

compiled CSS

397445b

localized base types

0a25f3d

wip

6198264

wip2

fe7afa4

ccjmk force-pushed the feature/79-hide-unidentified-item-details branch from 263f3be to fe7afa4 Compare November 5, 2023 18:09

ccjmk marked this pull request as draft November 5, 2023 18:12

arbron changed the base branch from 2.1.x to 2.4.x November 5, 2023 22:27

arbron added this to the D&D5E 2.5.0 milestone Dec 6, 2023

arbron removed this from the D&D5E 2.5.0 milestone Dec 19, 2023

arbron closed this Dec 19, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#79] Hide name/desc for items not identified #1696

[#79] Hide name/desc for items not identified #1696

ccjmk commented Aug 11, 2022 •

edited

Loading

aaclayton commented Aug 27, 2022

ccjmk commented Sep 1, 2022

Fyorl commented Sep 1, 2022

Fyorl commented Sep 1, 2022

ccjmk commented Sep 1, 2022 •

edited

Loading

ccjmk commented Sep 24, 2022

kaelad02 commented Sep 25, 2022

kaelad02 commented Sep 25, 2022

krbz999 commented Sep 25, 2022 •

edited

Loading

ccjmk commented Sep 27, 2022

arbron left a comment

arbron Oct 13, 2022

ccjmk Oct 13, 2022

arbron Oct 13, 2022 •

edited

Loading

arbron Oct 13, 2022 •

edited

Loading

arbron Oct 13, 2022

arbron commented Oct 13, 2022

arbron Oct 13, 2022

ccjmk Jan 28, 2023

arbron Oct 13, 2022

arbron Oct 13, 2022

arbron commented Oct 31, 2023

ccjmk commented Nov 2, 2023

	<input disabled name="ignored" type="text" value="{{ localize (dnd5e-unidentifiedName item) }}"/>
	<input type="text" value="{{ localize (dnd5e-unidentifiedName item) }}" disabled>

[#79] Hide name/desc for items not identified #1696

[#79] Hide name/desc for items not identified #1696

Conversation

ccjmk commented Aug 11, 2022 • edited Loading

aaclayton commented Aug 27, 2022

ccjmk commented Sep 1, 2022

Fyorl commented Sep 1, 2022

Fyorl commented Sep 1, 2022

ccjmk commented Sep 1, 2022 • edited Loading

ccjmk commented Sep 24, 2022

kaelad02 commented Sep 25, 2022

kaelad02 commented Sep 25, 2022

krbz999 commented Sep 25, 2022 • edited Loading

ccjmk commented Sep 27, 2022

arbron left a comment

Choose a reason for hiding this comment

arbron Oct 13, 2022

Choose a reason for hiding this comment

ccjmk Oct 13, 2022

Choose a reason for hiding this comment

arbron Oct 13, 2022 • edited Loading

Choose a reason for hiding this comment

arbron Oct 13, 2022 • edited Loading

Choose a reason for hiding this comment

arbron Oct 13, 2022

Choose a reason for hiding this comment

arbron commented Oct 13, 2022

arbron Oct 13, 2022

Choose a reason for hiding this comment

ccjmk Jan 28, 2023

Choose a reason for hiding this comment

arbron Oct 13, 2022

Choose a reason for hiding this comment

arbron Oct 13, 2022

Choose a reason for hiding this comment

arbron commented Oct 31, 2023

ccjmk commented Nov 2, 2023

ccjmk commented Aug 11, 2022 •

edited

Loading

ccjmk commented Sep 1, 2022 •

edited

Loading

krbz999 commented Sep 25, 2022 •

edited

Loading

arbron Oct 13, 2022 •

edited

Loading

arbron Oct 13, 2022 •

edited

Loading