Skip to content

Conversation

@RobinLefebvre
Copy link

In response to issue #209 , added a conditional within the selection styling logic in the Character Builder page. If the current selection is Eldritch Invocations (as defined by the 2nd element of option-path) and does not meet prerequisites (as defined by selectable?), then apply the "hidden" class.

Copy link

@KingMob KingMob left a comment

Choose a reason for hiding this comment

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

Hmm, this feels tricky. Um, what's in option-path? What guarantee is there :eldritch-invocations will always be the third item, if present?

For that matter, what guarantee is there that option-path will always have at least 3 items here? nth will throw an exception if it exceeds the bounds. If we're going to look up by index, get is safer, since it will return nil.

Sorry I can't check, don't have time to fire up everything at the moment...

@datdamnzotz datdamnzotz added area/application Task related to orcpub application itself enhancement New feature or request labels Jul 27, 2019
@datdamnzotz datdamnzotz added this to the 2.5.0.1 milestone Jul 27, 2019
@datdamnzotz
Copy link

This will be the breaking changes that @marloso2 wants to put in (really separates orcpub2 from this code) and will introduce compatibility differences between the sites.

@datdamnzotz datdamnzotz added the breaking-feature Breaks backward functionality with orcpub2.com label Dec 2, 2019
@marloso2
Copy link
Member

marloso2 commented Dec 9, 2019

This isn't actually a breaking feature. It just hides any Invocations that aren't selectable, it doesn't change anything about the files. Though, I'm not sure if that's a great idea either way so this should have a toggle, I think

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/application Task related to orcpub application itself breaking-feature Breaks backward functionality with orcpub2.com enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants