-
Notifications
You must be signed in to change notification settings - Fork 34
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
#123 Spell Info #126
#123 Spell Info #126
Conversation
Merge development
Will take a look at this next week. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me (though I didn't actually run the code). Just two things that look like they are leftover from a previous iteration or something.
src/components/AddSpellButton.js
Outdated
this.cur_character.setSpells(spell, level); | ||
}; | ||
|
||
_handleAccordionClick (ev) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Think this method is left over from a copy/paste and isn't used.
src/components/AddSpellButton.js
Outdated
this.spellDialog.open(); | ||
} | ||
|
||
_handleAddNewSpell (ev) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is getting used either? Which also means you don't need the getCurrentCharacter call in the connectedCallback.
Remove unused code
First draft of updated spell content. Looking for your opinion before I add more.
I was thinking a similar component with editable fields would work for the added spells.