Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added random pattern button that enables contrasting css patterns #31

Merged
merged 6 commits into from
Oct 27, 2023

Conversation

pbelokon
Copy link
Contributor

Closes #15
@manancodes could you please add hacktoberfest tags to this issue

I have added the toggle patterns button, when enabled shuffling would result in random background patterns.
For now I just added 4 patterns but I made the solution scalable, all you need to do it add pattern to the array of objects.

@manancodes
Copy link
Owner

Awesome work, can you fix the spacing here? Also, the button should be a toggle and not a normal button, which indicates whether it is enabled or not.

Screenshot 2023-10-21 at 2 12 10 PM

I will add the hacktoberfest labels once this solution is accepted and ready to be merged.

@manancodes
Copy link
Owner

Please check the mobile responsiveness as well.
Screenshot 2023-10-21 at 2 15 36 PM

@pbelokon
Copy link
Contributor Author

pbelokon commented Oct 21, 2023

@manancodes Just added toggle like effect when patterns are clicked and fixed it's margin on mobile. Is there anything else I should change ?

@manancodes
Copy link
Owner

manancodes commented Oct 23, 2023

Hey great work! but can you check it again? Spacing is still not fixed. This is how it looks on the desktop. Check the left and right side of the white container with all the controls.

Screenshot 2023-10-23 at 11 35 06 AM

Also, although the current implementation of the toggle button serves the purpose and fits with the UI. The toggle button is a bad UX implementation. Which will cause confusion. And it is not actually a toggle element.

A toggle button like this would be much better. With sharp corners and black colour. The size of the square toggle should be matched with the size of the square shuffle button that we have.
Screenshot 2023-10-23 at 11 38 17 AM

And a label on top saying patterns. Like the width label here:
Screenshot 2023-10-23 at 11 38 44 AM

Lastly, try to add screenshots in your PRs.

@pbelokon
Copy link
Contributor Author

@manancodes just fixed it here are the screenshots:

mobile
image
image

desktop
image
image

By the way I was not sure how mach spacing you wanted me to add so I just added a little let me know if you want me to change it.

@manancodes
Copy link
Owner

  • Media queries are always supposed to be at the last in a css file.
  • The left and right spacing should be equal to the top and bottom spacing.
  • Please try to match the spacing and sizing with other components to maintain consistency.
  • As requested earlier as well, The total width of the toggle container should be 80px, and the toggle switch should be 40px, to match the size of the shuffle button.
  • Please do not introduce new colors (green) for the toggle button, to maintain consistency. You can keep the toggle color to black only and change the color of the background from white to grey in the off position.

Please ask questions if you are not clear with something. Happy coding!

@pbelokon
Copy link
Contributor Author

Sure will do thanks for the explanation

@pbelokon
Copy link
Contributor Author

@manancodes I have moved all css components before media, made sure button and top have the same spacing, changed color and size of the toggle. Let me know if there is anything else you want me to change

image

image

@manancodes
Copy link
Owner

Great progress!
Although this looks much better and consistent to me. Using width 700px instead of 600px. Make sure to have equal spacing between all the controls. It is uneven right now.
Screenshot 2023-10-26 at 7 30 06 PM

Lastly, the toggle button is going out of view in the on position. Can you look into that again? It needs some adjustments.

@pbelokon
Copy link
Contributor Author

@manancodes I my latest commit I did change console width to 700px and fixed overflow of the toggle on. I did try to make spacing equal but I don't think it worked that well maybe create a new issue for that.

image
image

manancodes
manancodes previously approved these changes Oct 27, 2023
@manancodes manancodes merged commit 0439169 into manancodes:master Oct 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a feature to generate random patterns
2 participants