search

SscSPs / color

A single page application to view and play around with colors to find color palette
https://SscSPs.github.io/color
13 stars 29 forks source link

Colorgrid made responsive #37

Closed Snehil0603 closed 2 years ago

Snehil0603 commented 2 years ago

Fixes #17

Previously

p1 p2

Color Grid has been made responsive

r1 r2

SscSPs commented 2 years ago

Hey @Snehil0603, the screenshots looks great! Also, looks like there are some warnings, please fix them so we can go ahead and merge this

SscSPs commented 2 years ago

Awesome! Thanks for the changes! One suggestion I have,instead of hard coding them to 3%, we should set it based on the saturationCount and luminosityCount variables, The logic can be that we allow 50% for the grid, and then we can use the saturationCount and luminosityCount to get the value by dividing 50 by them!

So, something like this: width = 50/saturationCount Will that work?

Snehil0603 commented 2 years ago

Previously the height and width was being set using saturationCount and luminosityCount but it led to grid being un responsive that is why I set it to values .So it was not really working that way

SscSPs commented 2 years ago

Earlier, there were 2 datapoint being used, one was window's hight and width, and saturationCount and luminosityCount. Now you have removed both of them, and hard set it to 3% of window's hight and width, what I'm suggesting is to use 50(try a few different values, 60 or 70) instead of window hight/width, and use the resulting value as the size for cell in vh/vw, see if this works, I think it should work this way.

The reason I'm asking this is because there's another PR that adds support for modifying the count using a slider available to user, if we make this hard coded to 3, then changing the count may result in overflowing the cells form visible screen.

Snehil0603 commented 2 years ago

@SscSPs please kindly check this pr now