jb-tlm / color-charter

https://color-charter.vercel.app
MIT License
0 stars 0 forks source link

Fix the remove button sometimes not removing palette color #23

Closed jb-tlm closed 1 year ago

jb-tlm commented 1 year ago

If a key color is added to the palette, a click of the X removes it. However, if you choose secondary from the menu on the right, pick a secondary color, and add it to the palette at the top, pressing the X will remove the secondary color but will not remove the key color no matter how many times it's pressed. Investigate and fix this bug.

marcioreisjr commented 1 year ago

Hello Jacob,

The fix for bug "#9 - change add to clipboard button" highlighted the behavior you described. I tried to infer how the UI should work based on what I saw working before. If this is not how it should work, I suggest spending time documenting the entire interface. Wireframes and descriptions of the UI elements' actions can help save development time.

I have the bug "#13 - create color relationship buttons" on my list, and like a few others, it is an enhancement rather than a bug fix. Implementing this enhancement requires UI documentation. Could you provide me with the necessary documentation? I'm releasing bug #13 for now and will pick it up again when the time comes to implement it.

jb-tlm commented 1 year ago

Hey Marcio,

Thanks for all your work so far, you've been a huge help! This project was created by a current TLM student and has no documentation other than the readme. It was decided to provision this tool to replace the current broken color wheel in the LMS, but before that could move forward, certain changes were asked for. I volunteered to create the repo, create the issue tickets, ask for RC volunteers, manage the code review and test, but writing wireframes and descriptions for this entire project is a bit outside the scope of work that I have agreed to do. I'm trying to get this project to a point where the reviewers OK it to be put in the LMS for the students to take advantage of since their current tool is pretty much useless. The original list of 13 changes was what was asked for by the reviewers. The other 3 or 4 things I have added are edits or unexpected behavior that have resulted from these changes.

If there is an X next to a bar of colors and pressing it once deletes the most recent color, it's pretty intuitive that pressing it again would remove the next color, then the next, and next, and so on. For a remove button to only remove the secondary color but be unable to remove the primary color seems a bit confusing. Especially because that same X will remove color after color after color until there are none if the bar is full of primary colors. It shouldn't act one way with only primary colors, and then act completely differently if a secondary color is thrown in the mix. The remove button should be linked logically with the colors in the current palette, and it shouldn't matter how many there are, or whether they are primary or secondary. If there are colors in the palette, the X button should remove the most recent addition to that palette until there are no more to remove.

As for the documentation you're asking for regarding #13 - everything placed on the GitLab ticket by the reviewers is in the GitHub issue tickets. If there is something in one of the issues that you have questions about or need clarity on, please let me know and I am happy to take those questions to the specific reviewer who asked for that change and try to get us more detailed information. Again, I really appreciate your help, you have closed more of these issues than anyone else and I haven't found anything in your work that I thought needed to be changed. Your willingness to volunteer along with myself and the others to try to help the current and future students of TLM is commendable and appreciated.

marcioreisjr commented 1 year ago

Hi Jacob,

Thank you for getting this tool going for TLM. I am extremely grateful to TLM for all the support I got and that I'm still getting. I'm pleased to join this effort as a way to give back.

What you mentioned in the text is a good specification for the color bar delete button: "The remove button should be linked logically with the colors in the current palette, and it shouldn't matter how many there are, or whether they are primary or secondary. If there are colors in the palette, the X button should remove the most recent addition to that palette until there are no more to remove."

I will implement this behavior to fix bug #23 - "Fix the remove button sometimes not removing palette color."