primer / prism

A tool for creating and maintaining cohesive, consistent, and accessible color palettes
https://primer.style/prism/
MIT License
670 stars 40 forks source link

App crashes when we remove all colors from scale #10

Closed pavelkeyzik closed 2 years ago

pavelkeyzik commented 2 years ago

What

When I create a new palette and remove all colors from scale, I see a blank page

In Action

https://user-images.githubusercontent.com/17102399/173680962-4f7fcf31-4840-4fdc-86c3-bba25b766dec.mp4

P. S.

What about to add issue template, to let reporters know what they need to provide? 🤔

colebemis commented 2 years ago

Oof good catch. Thanks for the bug report!

Contributions are welcome if you'd like to try and fix this :) Also, feel free to open a pull request to add an issue template for reporting bugs

pavelkeyzik commented 2 years ago

I'd love to. But I need some time to learn how everything works 👍

colebemis commented 2 years ago

I'm happy to answer any questions! Also, apologies for the messy code 😅 Lots of opportunities for clean up

pavelkeyzik commented 2 years ago

That's okay! The idea is so cool, at least it works and it will be always something to fix. So, don't worry! I hope that community will be involved and help to fix everything 😊

rafaeltab commented 2 years ago

I know you're actively developing this now, but I want to note that this error can also be caused by clicking the 'Delete color' button on the right panel. And issue #19 has a very similar error, but when deleting the actively selected color only by clicking the - button, not the 'Delete color' button on the right panel.

pavelkeyzik commented 2 years ago

@rafaeltab Good catch 👍

pavelkeyzik commented 2 years ago

I'm going to close this issue as it's fixed as part of the #15

rafaeltab commented 2 years ago

This issue isn't completely solved yet, we can't delete the last color using the - button anymore but when using the Delete color button we can still delete the last color which results in a crash + an errored state.

The export file will look as follows if it becomes errored:

{
  "lightgrey": [],
  "dimgrey": [],
  "olivedrab": "#6b8e23"
}

Both lightgrey and dimgrey are bugged, olivedrab is not. When selecting a bugged scale, the page will also crash, meaning there is no way to delete the bugged scale either.

I believe the functionality that should be added is:

  1. The Delete color button should be unavailable and greyed out when there is only one color in a scale, exactly like the - button.
  2. If a bugged scale is opened, the app should not fully crash but instead display an empty scale, with the scale section of the right bar intact.
  3. (optional) The - and + buttons should still work on a bugged scale

I believe point two is the most important, people need to be able to fix their bugged palettes, instead of having to fully recreate them.

pavelkeyzik commented 2 years ago

@rafaeltab I think it'll be better to create a separate issue for this. We're going to fix a part of this issue here https://github.com/primer/prism/pull/37 but I've realized that user can import scale with empty array and we should add check for this as well 😊

rafaeltab commented 2 years ago

@pavelkeyzik I was going to make one after your comment on the other issue, but I really didn't have time today since I have to hand in my thesis. Tomorrow I'll be completely free and take the time to make one.

pavelkeyzik commented 2 years ago

Thank you @rafaeltab 😊