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

New stat tier editor #9992

Merged
merged 15 commits into from
Oct 30, 2023
Merged

New stat tier editor #9992

merged 15 commits into from
Oct 30, 2023

Conversation

bhollis
Copy link
Contributor

@bhollis bhollis commented Oct 24, 2023

This implements a new stat tier editor - feature flagged to beta for now.

Screenshot 2023-10-23 at 11 41 19 PM

Some features:

  1. More intuitive stat minimum selection. I know that leaving the minimums at all zero is the best, but people seem to deeply enjoy tweaking these. I see it as a way to search the space of stat tiers.
  2. Buttons for increasing/decreasing priority - I don't know how many people realize you can drag and drop, so these buttons are for them.
  3. Button for "lock stat" which sets the maximum to the minimum - aka "hit this exact stat tier". This makes up for the fact that I no longer expose the max-tier selector.
  4. Button for ignore/un-ignore, much easier than selecting from the dropdown.
  5. The sidebar is now wider to give this more room to breathe - it's conveniently the exact same size as the mobile viewport now.
  6. Full keyboard accessibility - focus the stat selector and use arrow keys or numbers to set stats.
  7. I did some work to make the reordering via button animate, but this doesn't work quite right - there's a little "jump" at the end. I had to jump through hoops to get it to animate at all, and I might yet remove this.

Very interested to hear ideas to make this more intuitive, more attractive, and most of all more useful for finding your optimal build.

This is only one more step in the revamp of LO - more is coming.

@robojumper

This comment was marked as resolved.

@lowPolySkeleton
Copy link
Member

I guess I don't really understand what the lock does even after reading what it does? So that's not super intuitive to me. Does it mean that if I select 5 on the stat bar it won't try to hit 5? It'll try to hit from 5-10? Unless I hit the lock?

Why not just let the selection do that? So it's not a bar from 0 it's whatever you want? Could be a highlight from 3-5 instead?

I guess there is no way for someone to know that's a minimum they are selecting... to me that interface reads as "I pick this stat".

@guise
Copy link
Contributor

guise commented Oct 24, 2023

👀 looks great!

Some initial thoughts - mainly UI / interaction:

  • Tier stats
    • The tooltip often blocks stat selection when quickly moving the mouse left → right. Making the tooltip appear above/below might avoid this?
    • Tiers without matches have a heavier visual weight than tiers with matches; greying these out feels like a more intuitive way to show the effective ceiling, but would also need tweaking selectable tiers to differentiate them a bit
      The counter-point: it risks making these tiers look disabled but they're still clickable which could be confusing. Rough mockup / suggestion:
      image
  • Priority buttons
    • Good for a11y 👍🏽
    • May be worth adding a hover (primary accent) to help delineate them since they're packed tightly together
  • Ignore button
    • Semantically this feels like it should be 'Include' since stats are only included when this is toggled on
    • A more traditional leading checkbox style would be clearer here IMO (ticked for on/ empty for off) — currently the on/off icons are very similar at a glance because the solid fill is the only differentiator
      • Sidenote: personally i'm not a fan of circular checkbox styles in general since they encroach on radio button styles (but i'm conscious we use these circular styles in a few places - column picker in the organizer, and item sort display settings)
    • Mockup/idea for left-aligned checkboxes whilst keeping drag handles beside the stat names (this keeps the middle section as the largest drag-able area of a row)
      image
    • (Not shown in mockup) Moving the lock icon to the far right could further clean up the interactions since the most common actions (include stat + lock tier) should be easier to toy with at the far left/right extremities of each row
  • Re-ordering priority
    • Potentially unnecessary combination recalculations:
      • Re-ordering ignored stats
      • Re-ordering included stats above/below ignored stats, but it's order remains the same among only the included stats
      • Re-order a stat into it's original order position
      • Re-ordering using increase/decrease buttons: combinations are re-scanned as soon as you click a button which "locks" the UI, preventing you from chain-clicking multiple priority buttons — a short-delay before recalculating combos explicitly when reprioritising using these buttons could alleviate this
        • Possibly related to the animation… re-ordering using the buttons is significantly slower than re-ordering by dragging a stat (haven't profiled this but it feels ~5x even though the 'generated in XX seconds' shows the same time)
    • Idea: (although this could be more annoying than helpful) — dragging an ignored stat above an included one could automatically include it, removing a manual step to include that stat
      • If you're explicitly increasing the priority of an ignored stat above any included stat it's fair to assume you want to include it
      • This doesn't seem important if you're de-prioritising stats (i.e. they could stay included if you drag them down)

@guise
Copy link
Contributor

guise commented Oct 24, 2023

I guess I don't really understand what the lock does even after reading what it does?

I'd echo this. I had to re-read and toy around with it before getting my head around it. From what I understand:

  • By Default/unlocked = matches "at least" the selected tier (min: selected, no max)
  • Locked = matches "equals" the selected tier (min: selected, max: selected)

This could be better explained with improved UI text, to a degree, but the UI itself could be more intuitive.

Automatically 'locking' a stat at T10 is also a little confusing — mainly because a stat can't be 'unlocked' and the tooltip text is the same.

Idea (although i'm not sure I like this myself): replace the lock icon with logical operator icons to indicate how each stat is being evaluated (/=) when toggling.

Another potential alternative was to consider highlighting the selected tier → 10 in orange (instead of 0 → selected) — and only the selected tier if a stat is locked. However, since we visualise whether stat tiers have matching armor combinations this would likely lose that detail.

Wild card option: get rid of the lock entirely? Always match "at least" on the selected tier and rely on the specified order for further prioritisation. The obvious risk here is removing functionality that people use — but i'm struggling to think of scenarios where locking gives you a benefit over not-locking?

@bhollis
Copy link
Contributor Author

bhollis commented Oct 24, 2023

The way it was explained to me is that some folks consider the last few tiers to be useless, either because they don't provide a significant enough benefit, or because they have exotics or mods that will make up the difference when activated. So they want to prevent LO from putting extra points there. The correct move would be to reduce the priority of that stat but editing priority is difficult for folks to grasp.

@bhollis
Copy link
Contributor Author

bhollis commented Oct 24, 2023

Does it mean that if I select 5 on the stat bar it won't try to hit 5? It'll try to hit from 5-10? Unless I hit the lock?

Yup - that's how it works today if you select Min 5, Max 10. Hitting the lock is equivalent to setting it to Min 5, Max 5.

@bhollis
Copy link
Contributor Author

bhollis commented Oct 26, 2023

Re-ordering using increase/decrease buttons: combinations are re-scanned as soon as you click a button which "locks" the UI

The "locked" UI was a bug where I had too long a timeout for the react-dnd move operation - leftover from some testing.

dragging an ignored stat above an included one could automatically include it, removing a manual step to include that stat

This is how it used to work a long time ago! I should really bring it back.

@bhollis
Copy link
Contributor Author

bhollis commented Oct 26, 2023

@guise I implemented a bunch of your suggestions - I think it's a lot cleaner now. I tried using >= and = instead of the locked icon, but in the end decided to just remove it entirely. We'll see if anyone complains.

Screenshot 2023-10-25 at 11 50 08 PM

@guise
Copy link
Contributor

guise commented Oct 26, 2023

Few more thoughts/suggestions

  • I think it's worth keeping/reinstating the drag handles as they're such a strong affordance for indicating draggable objects
  • Using / arrows (with stems) differentiates the order icons from regular 'disclosure' arrowheads;
    • In most other contexts ˄/˅ icons indicates disclosure of some kind (show dropdown or expand)
    • As a design pattern, stemmed arrows would also work for item sort ordering in Settings
  • Stat container bg should be rgba(0, 0, 0, 0.4) to match the other LO panels
  • If folks want locking reinstated, a clearer alternative could be a labelled toggle which feels more self-explanatory (see mockup below)
    • Min/Exact could be squished into a fixed-size toggle; important so it stays the same size when toggling between values
    • Min/Max would look cleaner, although 'max' is technically inaccurate it might "feel" true/obvious, particularly in the scenario you mentioned where people are using it to explicitly set a ceiling to accommodate temporarily buffed stats
      • Presumably this would translate better too
    • This toggle could potentially be made into a dropdown (where the suggestion for stemmed arrows originated) — but that feels like it would be nesting too many interactions

image

@bhollis
Copy link
Contributor Author

bhollis commented Oct 27, 2023

I think it's worth keeping/reinstating the drag handles as they're such a strong affordance for indicating draggable objects

Added them back.

Using ↑/↓ arrows (with stems) differentiates the order icons from regular 'disclosure' arrowheads;

Good call - changed them in both places, and applied some of the other design updates to the sort order editor.

Stat container bg should be rgba(0, 0, 0, 0.4) to match the other LO panels

I don't mean for these to be the same as the other LO panels which are containers for controls - these are themselves controls. I'd love advice on how to clean up and clarify those other sections, but giving the stat rows a transparent background made them too indistinct.

Screenshot 2023-10-26 at 11 34 41 PM Screenshot 2023-10-26 at 11 34 47 PM

@guise
Copy link
Contributor

guise commented Oct 27, 2023

Gotcha. My main concern with the opaque black bg is that it's jarring on some themes. A darker tint (0.6) seems to work well with some other changes…

We can adjust spacing to group controls more naturally into sections:

  • Increase the space between sections (stats / armor / mods / subclass) — this space becomes the main horizontal break between sections
  • Decrease space within sections
    • Remove the gap on the flex layout for the column as a whole
    • 1px between stats
    • Condense padding/margin/gap within stats
    • Adding a small left indent for the tier control prevent it from creating an optical horizontal break within an enabled stat

Rough mockup showing before/after spacing tweaks (note: this spacing is probably too tight)

image


It's also worth disabling the tier tooltips during a drag — atm they can appear mid-drag causing the tooltip to appear infront of the stats you're re-ordering

@bhollis
Copy link
Contributor Author

bhollis commented Oct 28, 2023

Thanks, I'll file those spacing ideas as a followup. Though I don't think I can remove the gap from between the sections - your mockup puts Assume Masterwork, Include Vendor Items, and the Exotic selection together, but they're all independent panels.

@bhollis
Copy link
Contributor Author

bhollis commented Oct 28, 2023

OK I've prevented presstips (everywhere) from triggering if you hover while a mouse button is down. Should be a nice improvement for some edge cases.

Anything else I need to touch before this can go in?

@bhollis bhollis merged commit 524d50b into master Oct 30, 2023
5 checks passed
@bhollis bhollis deleted the stat-editor branch October 30, 2023 21:14
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.

4 participants