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

[#1] New Tidy compatibility update #2

Merged
merged 2 commits into from
Mar 29, 2024

Conversation

kgar
Copy link
Contributor

@kgar kgar commented Mar 29, 2024

#1

Removed Tidy wire-up from the standard render path.

Just a recommendation: changed render(true) to render(false) to prevent unexpected behaviors when the user is updated. There are a variety of reasons why the user will update in the current version of Foundry dnd5e.
@@ -139,6 +204,6 @@ function removeInspiration(user, _sheet) {

function updateSheetForInspo(user) {
if (user?.character.sheet.rendered) {
user.character.sheet.render(true);
user.character.sheet.render(false);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mysurvive this is mostly just a recommendation.
render(true) can have some surprising effects when it fires off for each user change.
Tidy and the new Default Sheets can frequently have user changes that do not relate to this module which can be triggered from actions within the sheet, such as changing the sort on various tabs.
I tested it and noted that the legacy sheet and Tidy were refreshing successfully with a render(false).

@@ -23,6 +23,75 @@ Hooks.on("updateUser", (user) => {
updateSheetForInspo(user);
//updatePlayerListInspo();
});

Hooks.on("tidy5e-sheet.renderActorSheet", (app, element, data) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because I'm running things in svelte, I have to post up my own lifecycle hook.

inspirationArea.hide();

let currentInspiration;
const actorOwner = game.users.find(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For this PR, I just copied a lot of the functionality from the main logic. Please feel free to extract the common behaviors into functions if that works better for you.

>
<i
class="fa-solid fa-plus"
style="color: var(--t5e-primary-font-color)"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This CSS variable reference makes So Inspired! work with Tidy's light and dark modes 💪💅

@mysurvive mysurvive merged commit 6b55642 into mysurvive:main Mar 29, 2024
2 checks passed
@mysurvive
Copy link
Owner

You're fast. I was trying to push the linting issue fix and was wondering why my branch was behind :)

@kgar kgar deleted the tidy-5e-compat-update branch March 29, 2024 21:17
@kgar
Copy link
Contributor Author

kgar commented Mar 29, 2024

Oh yeah, sorry about that! I happened to see it while I was at a computer.
Hey, thanks for working with me on this.

@mysurvive
Copy link
Owner

Absolutely, glad people are enjoying the module. My DM asked if I could make a SWADE/PF2e style inspiration module for our game - didn't expect anyone else beside my DM to use it :)

You can send me a message on Discord if you need anything else (or on here is fine, of course) - mysurvive

Resolves #1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants