search

finos / a11y-theme-builder

DesignOps toolchain theme builder for accessibility inclusion using Atomic Design.
Apache License 2.0
46 stars 69 forks source link

[SDK] allow deleting the default color #979

Open omesh-omg opened 4 months ago

omesh-omg commented 4 months ago

Problem/Concern : cannot delete default color

Required in progress with the EPIC - 820 #820

Uncaught runtime errors: × ERROR You can not remove the default color (Color Palette) at ColorPalette.removeColor (http://localhost:3000/static/js/bundle.js:90920:13) at handleMenuButtonDeleteColor (http://localhost:3000/static/js/bundle.js:5485:32) at onClick (http://localhost:3000/static/js/bundle.js:5693:40) at HTMLUnknownElement.callCallback (http://localhost:3000/static/js/bundle.js:172881:18) at Object.invokeGuardedCallbackDev (http://localhost:3000/static/js/bundle.js:172925:20) at invokeGuardedCallback (http://localhost:3000/static/js/bundle.js:172982:35) at invokeGuardedCallbackAndCatchFirstError (http://localhost:3000/static/js/bundle.js:172996:29) at executeDispatch (http://localhost:3000/static/js/bundle.js:177139:7) at processDispatchQueueItemsInOrder (http://localhost:3000/static/js/bundle.js:177165:11) at processDispatchQueue (http://localhost:3000/static/js/bundle.js:177176:9)

Proposed Solution

aaronreed708 commented 3 months ago

Do you mean that the color is the first color ever added to the palette? Or is it because you are trying to delete the last and only color left in the palette?

omesh-omg commented 3 months ago

It is the first color added to the palette .

If it is last color we will deny the user that it is not possible to delete.we don't have to use SDK at all in that case

aaronreed708 commented 1 week ago

So I looked into this today. I don't see a reason that we can't call colorPalette.removeColor on the default color. In fact, I don't see a need for it at all. The only place of significance where I see it used is in the documentation (https://a11y-theme-builder.finos.org/designers/how-to-work-with-tokens/#json-structure) where it just calls it out while giving an example of the structure of the color palette if you are looking at the design tokens. And since I don't see a corresponding property in the generated JSON in the Code tab, I'm not sure if the JSON in the documentation is still valid or not. Since I don't know why the defaultColorName was tracked in the first place, I'd hate to guess what the behavior should be if we delete it and there are other colors remaining in the palette. How would you know what the new default color should be?

@lwnoble the design token sample in https://a11y-theme-builder.finos.org/designers/how-to-work-with-tokens/#json-structure does not match what is currently being output from the JSON Generator. I assume that this is something that has changed since the documentation was written? The generated JSON currently doesn't have the colorPalette.defaultColorName property anywhere, nor even the colorPalette object. Do you know any reason why we'd need a defaultColor in the color palette anymore? If not we are going to toss it. Let us know your thoughts. Thanks!

aaronreed708 commented 4 days ago

spoke to @lwnoble in today's call, this is no longer referenced in the generated JSON so can be safely deleted by a user.