Conversation
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/vercel/og-image/jmit3yucz |
rfoel
left a comment
rfoel
left a comment
There was a problem hiding this comment.
You might want to make the button wider in this case
The calc + 2px is to account for inconsistency with the other inputs and selectors which appear to be 2px wider than 100% (presumably from border)
styfle
left a comment
styfle
left a comment
There was a problem hiding this comment.
Thanks for the PR! 🎉
I left a small suggestion
The array has to be spread before filtering. This is for the case where only a later image has a defined width/height In those cases the width/height array would be defined as [4: 350] for example. Filtering on that array would give unexpected result. The spreading creates the full array and then filters correctly.
| label: `Remove Image ${i + 2}`, | ||
| onclick: (e: MouseEvent) => { | ||
| e.preventDefault(); | ||
| const filter = (arr: any[]) => [...arr].filter((_, n) => n !== i + 1); |
There was a problem hiding this comment.
| const filter = (arr: any[]) => [...arr].filter((_, n) => n !== i + 1); | |
| const filter = (arr: string[]) => arr.filter((_, n) => n !== i + 1); |
There was a problem hiding this comment.
Happy to correct the type.
The spreading of the array is needed however; if for example a user only sets the width/height of the 4th image it will make those arrays defined as [4: 350]. The suggested method leads to the width/height arrays being filtered incorrectly, as the leading undefineds are not created .
See https://codepen.io/spinks_/pen/xxVdPBN for how the two filters apply would apply in this case.
At least this is what I found testing it, doing it with the suggested method leads to the widths/heights being moved around incorrectly due to that behaviour.
* Add button that removes an image * Make remove button 100% width The calc + 2px is to account for inconsistency with the other inputs and selectors which appear to be 2px wider than 100% (presumably from border) * Make splice/filter operation clearer The array has to be spread before filtering. This is for the case where only a later image has a defined width/height In those cases the width/height array would be defined as [4: 350] for example. Filtering on that array would give unexpected result. The spreading creates the full array and then filters correctly. Co-authored-by: Steven <steven@ceriously.com>
3 participants
Hi this adds the ability to remove images after they are added, this would close #129.
Have done a good amount of testing and works correctly consistently, including cases where there may be empty width/height arrays.