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

[#4787] More informative roll dialog titles #4884

Open
wants to merge 5 commits into
base: 4.2.x
Choose a base branch
from

Conversation

roth-michael
Copy link
Contributor

@roth-michael roth-michael commented Dec 15, 2024

Closes #4787
Also added a check when putting the subtitle for an activity use in a chat card whether the activity was a heal-type, if so display "Healing Roll" instead of "Damage Roll."
Attack Roll:
image
Damage Roll:
image
Skill Check:
image
Ability Check:
image
Saving Throw:
image
Tool Check:
image
Fixed subtitle for a heal activity:
image

Haven't tested much with super long item/actor names. Might make sense to put them as subtitles instead of titles.

@roth-michael
Copy link
Contributor Author

Might instead make sense to put the roll type in the main title and the actor/item name in the subtitle (if it's okay to lose "Configure Roll" - which I think it would be, since it's implicit).

@arbron arbron added the ui User interface related features or bugs label Dec 15, 2024
@roth-michael
Copy link
Contributor Author

New commit changes to match my last comment.
Skill check:
Screenshot 2024-12-15 190904
Tool check:
Screenshot 2024-12-15 191015
Damage & Healing rolls:
Screenshot 2024-12-15 190952
Screenshot 2024-12-15 191004

@arbron arbron added the system: dice Dice rolling functionality label Dec 20, 2024
@arbron arbron added this to the D&D5E 4.2.0 milestone Dec 27, 2024
@arbron
Copy link
Collaborator

arbron commented Dec 27, 2024

@roth-michael Give a comment on the original issue so I can assign it to you

Copy link
Collaborator

@arbron arbron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good. Might also be nice to show "Concentration Save" when rolling for concentration.

module/documents/activity/mixin.mjs Outdated Show resolved Hide resolved
module/documents/chat-message.mjs Outdated Show resolved Hide resolved
@roth-michael
Copy link
Contributor Author

Might also be nice to show "Concentration Save" when rolling for concentration

Went with the same label but with Concentration instead of Strength for instance (i.e. "Concentration Saving Throw"). Was thinking about whether instead "Concentration Save (Con)" would make sense, but as-is it looks fine to me.
image

@roth-michael roth-michael requested a review from arbron December 30, 2024 19:36
Copy link
Collaborator

@arbron arbron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just the small change about where we set the concentration label, but otherwise looking good.

module/documents/actor/actor.mjs Outdated Show resolved Hide resolved
@roth-michael roth-michael requested a review from arbron December 30, 2024 20:02
@arbron arbron requested a review from Fyorl December 30, 2024 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
system: dice Dice rolling functionality ui User interface related features or bugs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Roll dialogs no longer have contextual window titles
2 participants