Turnip-Calculator icon indicating copy to clipboard operation
Turnip-Calculator copied to clipboard

Add support for tracking multiple sets of turnip prices

Open JonathanGin52 opened this issue 4 years ago • 2 comments

What is this PR trying to do?

  • Closes #88

This PR adds support for tracking multiple sets of turnip prices in parallel. For simplicity, each set of turnip prices is referred to as an "island". The changes include:

  • Introducing tabs to navigate between islands
  • Use a unique localStorage filter key for each island
  • Store tab list in localStorage
  • Support for dynamically adding / removing tabs

Demo

tabs

Risks

  • I'm no desiginer by any stretch, so this isn't the prettiest implementation 😬
  • Delete tab icon a bit finicky, sometimes when clicking on the icon, nothing happens. I believe this is an issue with material-ui
    • No specific steps to reproduce, but you can be "too accurate" when clicking, and actually click the nested path component, which doesn't appear to trigger the onClick event image

Next steps (not covered in this PR)

  • Support naming tabs
  • Confirmation modal before deleting tab

JonathanGin52 avatar May 24 '20 04:05 JonathanGin52

I will review it, and let you know. Thank you a lot!

elxris avatar May 25 '20 23:05 elxris

Ok, so I was going through this and some thoughts I collected.

Far from design, I think the following only applies to UX, delete button shouldn't be that close to the tab, and not without a warning that all your data is going to be deleted.

Stretch goals:

  • Maybe let the users rename the tabs?
  • Reorder tabs

I also think this an "advanced" feature and we should have like a small panel to enable this features and more to come.

We should have special care for users with data, we can't just delete or not "migrate" their old data.

elxris avatar May 30 '20 20:05 elxris