HeartofPhos / exile-leveling

Path of Exile Leveling Guide
https://HeartofPhos.github.io/exile-leveling/
159 stars 31 forks source link

Add skill link preview in sidebar #57

Closed ennukee closed 1 year ago

ennukee commented 1 year ago

See updated data and views in the comments below

Preconditions: User must sync their tree name with the gem section name. For example, image image

Lingering questions:

Current UX:

HeartofPhos commented 1 year ago

Generally for the idea, would be very nice to show links in a easy to access place. I want to avoid relying on POB "standards" to surface these kinds of features, simply can't rely on build makers to be consistent and would hide otherwise very useful features from anyone not in the know.

A better solution might be assigning skill groups to a passive tree in the Build section, at least exposes the feature to any user and has similar to manually arranged a non conforming POB

The intelligence skill gem color looks like it has a bit of bad contrast against the dark background, is that fine?

This can probably be solved by using the same coloured circle gems use in the route itself e.g image

First gem in a link is bolded and unindented Secondary gems (normally supports) are normal font weight and indented to indicate their connection to the primary gem

Can fairly easily data mine which gems are supports and then consistently list active gems first and supports indented after, happy with the format as is otherwise

ennukee commented 1 year ago

Can you expand a bit more on the first part? Is that to say that instead of relying on syncing the tree names and gem section names, we would just always display the gems and have controls similar to the build tree ones to cycle through the different gem sections?

ennukee commented 1 year ago

edit: I am off to bed, so please feel free to make any additional changes in the meanwhile.

I've made the following changes to try and best meet what you were mentioning:

Additionally, I added a toggle to the Build edit page (like the gems-only one), which toggles whether or not to show the gem links in the sidebar. I added this because while a PoB may have good, sectioned leveling trees, it could have a crazy gem link setup which would cause a lot of visual clutter the user may not want.

Current UX: image

Matches the following: image image

HeartofPhos commented 1 year ago

I've added is_support to the gem data and changed the filtering to happen at import to simplify the component a little. Also updated styling so that support gems without any active links use the same styling as active gems (some POBs like https://pobb.in/hPGOSBeJcMvV level support gems early for example)

Also put a max-height on gemLinkSection and moved the nav buttons to prevent things jumping around as you click

Let me know if there's anything you disagree with/think needs to be added

ennukee commented 1 year ago

Looks good to me from what I can see overall.

A couple notes I had when looking at it visually:

Example of my act 1 setup, for the first point I raised (there are 3 additional skills under Frostblink): image

ennukee commented 1 year ago

I went ahead and tested a few numbers but felt like doubling it to 20rem for max-height felt the best for me personally. It should allow for around three 4Ls to be fully visible, or two 4Ls and several auras / utility spells

image

My other concerns that I mentioned are minor and up to you if you want to modify them at all. Otherwise, I think I'm good with the changes unless you have any other concerns.

HeartofPhos commented 1 year ago

I agree 10rem is a too small, honestly the sidebar is becoming overly busy and is begging for a refactor. As a band-aid I've set the min-height to 15rem to give a bit of breathing room for search strings.

Also moved the gem links below search strings to avoid things jumping around, not an ideal solution and definitely need to plan out a better way to display sidebar content, but that's out of scope for this PR