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

Tidy Compatibility, Refactors, Bugfixes #46

Merged

Conversation

kgar
Copy link
Contributor

@kgar kgar commented Jan 24, 2024

Related to #30

Item Sheet: Magic Item tab

  • Remove any references to Tidy Kgar from magicitemtab.js; this tab class will likely pertain only to the default and default(like) sheets.
  • Push show/hide enable/disable logic to the templates
  • Eliminate unnecessary magic item tab event handling (anything in the template with a name attribute saves itself on change)
  • Eliminate manual calls to render wherever able. Changes to MI tab data will trigger a re-render naturally.
  • Manually update the Item in certain cases. Things like delete buttons need to manually update the Item after adjusting MI data to remove the spell.
    • dropping on a new spell/feat/table
    • clicking to delete a spell/feat/table
  • Fix radio button issue
  • Proposed change: Leverage HTML structure of "Items with Spells" for the lists of Magic Item spells/feats/tables. It uses default styles naturally.
    • Here's what all would change: 915c755

General/Misc.

  • Evaluate current practices like the magic item instance caching. From what I understand, the Magic Item class is an adapter to surface flag-based values in a useful way
    • I understand what's going on now. Basically, I can spin up a MagicItem anytime, as long as I have an Item5e document. At least in the Item tab, it is not really meant to be long-lived.
  • Resolve NaN's for the default sheets

Tidy Compatibility

  • Register/configure Magic Item tab for item sheet
  • Add "Magic Item" wand icon to relevant inventory rows
  • Add Spellbook section for magic items
  • Add Features section for magic items
  • Handle MI2 events on Tidy sheets
    • This will require making some of the event handling code static (or simple functions with parameters) so that it can be shared.
  • Proposal: Use default-sheet-style HTML for the list tables, same as with the Magic Item Tab in the Item Sheet.

Wrapping Up

  • Default Sheets have the expected MI2 behavior
    • Item Sheet
    • Actor Sheet
  • Original Tidy Sheets have the expected MI2 behavior
    • Item Sheet
    • Actor Sheet
  • New Tidy Sheets have the expected MI2 behavior
    • Item Sheet
    • Actor Sheet

…ator to allow prettier to format without error.
…g. Made content event handling a static/shareable function to allow for alternate sheet integration.

Removed all unnecessary event handling for the MI tab. Added manual item updating to spell/feat/table delete icon click events. Ensured the MI tab stays selected between renders when interacting with delete icons.

Added manual item updating when dropping a relevant entity onto the MI tab.

Began to extract `onDrop` for more public shareability.
…lears all data on construction if it is disabled.
…without the MagicItemTab class.

Made disableMagicItemTabInputs static so that it can be shared with other sheet modules without the MagicItemTab class.

Revamped tidy registration to leverage tab contents event listeners, on-drop handling, and tab contents input disabling during onRender callback. Added getData prop for the Handlebars template. Formalized the isAcceptedItemType and isAllowedToShow static functions in MagicItemTab to be shared between Tidy and regular tab handling.

Replaced Tidy5eKgarItemSheet constructor detection with API call that checks whether MI2 is dealing with a Tidy sheet.
Made sorting universally applied upon MagicItem construciton, so it is no longer necessary to call on render in MagicItemTab.
…c that maintains focus on the Magic Item tab. The fix was to use a fresh copy of `app` each time something is being done. Otherwise, a previous version of app will be cached in the tab, and all tab activations will be called on a sheet that is no longer being used.
@kgar
Copy link
Contributor Author

kgar commented Jan 24, 2024

I got to a good stopping point for the day. If you are interested in pulling these changes in or testing out what I've done here, please feel free.

Tomorrow, I can continue to work on Tidy integration with the Actor sheet.

@kgar
Copy link
Contributor Author

kgar commented Jan 25, 2024

This will possibly fix #37 also

Converted the item lists for Magic Item tab to use the standard/default sheet HTML structure/classes. This eliminated the need for a number of CSS styles.

Added some minimal layout styles for the item lists.

Removed unneeded template content for magic item tab, based on refactors.
Added scaffolding for placing the Wand icon and wiring events for the MI2 content.
Added magic wand for visible magic items logic for New Tidy.
Removed some commented code.
Some light refactor/renaming for clarity.
@kgar kgar changed the title (DRAFT) Some refactors for MI2 (DRAFT) Some refactors for MI2 / Tidy Compatibility Jan 26, 2024
kgar and others added 3 commits January 25, 2024 21:46
While this is not needed for default-like sheets, because they are slotting into an existing ordered list, for a non-default-like sheet, there may not be a parent ordered list to drop into.

Removed d20 from feat and spell sheet templates, as it is not needed. Also removed the CSS style for showing/hiding the d20 icon, because dnd5e sheets are already doing this.
Wired up Tidy sheet events with the new statically available version of the sheet event handlers.
@PwQt PwQt added this to the 1.5.0 milestone Jan 26, 2024
…look and feel in Old Tidy was compromised. Added some emergency patch styles to make Old Tidy presentable until it retires.
…k handler is doing a half-second `setTimeout()` to build items. New Tidy needs thos built items synchronously during render.
- Magic Item requires Equipped and Attuned
- Item Attunement is set to "Attunement Not Required"
- Item is equipped by Actor
@kgar kgar changed the title (DRAFT) Some refactors for MI2 / Tidy Compatibility Tidy Compatibility, Refactors, Bugfixes Jan 26, 2024
@kgar kgar marked this pull request as ready for review January 26, 2024 22:56
@PwQt
Copy link
Owner

PwQt commented Jan 27, 2024

Seems to be working and not breaking any compatibility, there are still few quirks (mostly visual) that I'll fix later.

Thanks for the gargantuan amount of work you did 😅 .

@PwQt PwQt self-assigned this Jan 27, 2024
@PwQt PwQt added the module compatibility It's about other module compatibility label Jan 27, 2024
@PwQt PwQt merged commit 9a67212 into PwQt:30-tidy-5e-sheet-rewrite-compatibility Jan 27, 2024
@kgar kgar deleted the 30-tidy-kgar-refactors branch January 27, 2024 16:44
@kgar
Copy link
Contributor Author

kgar commented Jan 27, 2024

Thanks for letting me participate. If you ever want to chat about Foundry dev, I'm eager to discuss with other module devs, as I am still learning Foundry dev and techniques for building modules.

MI2 is very special to a lot of users. Some of my community members have been asking me about compatibility for a while now. I'm just glad that I could finally get New Tidy to a place where it could offer the appropriate tools for MI2 integration.

p4535992 has a commission for more work in MI2, and I'm interested in doing the work for that, however long it takes, as long as you're ok with that.

@PwQt
Copy link
Owner

PwQt commented Jan 27, 2024

Sure, as I've written previously, my history with this project is just having it working on Foundry v11, for me and my friends, so I did some minor fixes that took me way too long to figure it out. I also only have time to try and do some things on it during the weekends, since after work I don't really want to look at any code.

And I haven't really worked in any JavaScript-like languages, so that's another barrier of problems

And that's how we ended up here 😅 - any help is appreciated.

@kgar
Copy link
Contributor Author

kgar commented Jan 27, 2024

If you ever want to chat on discord, I'm there on the league server and on a few others, like the dedicated Tidy discord server (link on the repo readme). I'm glad to assist in gargantuan ways haha and will defer to your direction on things. I understand the module inner workings a lot better now that I've been in there a bit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module compatibility It's about other module compatibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Deletion of Settings on Edit Tidy 5e Sheet Rewrite Compatibility
2 participants