lospec / pixel-editor

An online canvas based Pixel Art creation tool for Lospec.com
https://Lospec.com/pixel-editor
GNU General Public License v3.0
923 stars 74 forks source link

Fixed removal of all the colors from palette #114

Closed itsamrit closed 1 year ago

netlify[bot] commented 1 year ago

Deploy Preview for competent-tesla-4b5f1e ready!

Name Link
Latest commit 153f0366d4aa588ed4ebaace4cf621bec365d925
Latest deploy log https://app.netlify.com/sites/competent-tesla-4b5f1e/deploys/63b434746974f7000812acf4
Deploy Preview https://deploy-preview-114--competent-tesla-4b5f1e.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

unsettledgames commented 1 year ago

Thanks for the PR! The code right now feels a bit redundant. Can't you just subtract one from endIndex if the user is trying to remove all the colours?

itsamrit commented 1 year ago

Okay

itsamrit commented 1 year ago

I have made the changes. Please look at it. If there any problem from my side, please tell me. I'll make sure to fix it.

unsettledgames commented 1 year ago

Hey, thanks for the attempt. I may not have properly explained myself, what I meant is, if the user is trying to delete all the colours, delete ALL of them except the last one. To do so, it's probably enough to keep the old code, check when the user is trying to delete all the colours and decrease endIndex by 1.

itsamrit commented 1 year ago

If we just check, if user is trying to remove all colors then endIndex minus 1. Then there will be bug left : eg: if there is 4 colors in palette & user selects all colors to remove then endIndex comes to 2nd index from 3rd index by minus 1 ,but if user clicks 'remove' again without selecting anything then startIndex is still on 0th index & endIndex is at 2nd which causes removal of all colors from palette. To fix this i have used condition : if(coloursList.childElementCount == 1) return;

unsettledgames commented 1 year ago

Looking perfect, I'm merging this! Thanks 👍

itsamrit commented 1 year ago

Thanks 👍👍