Israel Adefidipe
@iadefidipeAll comments
- @OluwajuwonomoyeleSubmitted over 2 years ago@iadefidipePosted over 2 years ago
Hi @Oluwajuwonomoyele,
Great implementation you have here. Just a few improvement I can point out.
- You should give the desktop view login button a transparent border, so it doesn't shift the layout when it is Hovered
- In the features section, you should add more space between the section's svg and the details, so they don't begin to overlap
- Optional: The FAQ section, only one accordion section should be opened at a time, so the layout is a bit tidy
0 - @RodCriSubmitted about 3 years ago@iadefidipePosted about 3 years ago
Great work @RodCri! but you have a lot of improvements, I think you could make:
- You work isnt responsive on smaller screen
- the faq accordion can be improved on by ensuring once any section of the accordion is opened others are closed, no two sections can be opened at the same time. I think it will make for better user experience
- You should also work on the last FAQ section Cheers🐱👤
0 - @akalizk113Submitted about 3 years ago@iadefidipePosted about 3 years ago
Great work @akalizk113! Just two improvements, I think you could make:
- the placements of the page illustrations is not according to the design specification, or it might be my browser. Your hero section appears not to be not flexed in the desktop view, but the solution screenshot it looks flexed. Maybe you should check your work across different browsers
- the faq accordion can be improved on by ensuring once any section of the accordion is opened others are closed, no two sections can be opened at the same time. I think it will make for better user experience. Other than that, your implementation appears to be perfect. Cheers🐱👤
Marked as helpful1 - @GhostemaneUrsSubmitted about 3 years ago@iadefidipePosted about 3 years ago
hi @GhostmaneUrs, your implementation is great. The page overflows on smaller views, you might want to check that, by probably giving the page a max-width. Not really important, but the buttons styles should also have transition time
0 - @MartinLundqvistSubmitted about 3 years ago@iadefidipePosted about 3 years ago
hi @MartinLundqvist great implementation. Your project only seems to have spacing issues, i feel you can do better with the spacing in the desktop view. The cards are not centered in the mobile view and the header also has some padding issues
Marked as helpful0 - @thekindbardSubmitted about 3 years ago
izi pizi
@iadefidipePosted about 3 years agoGreat Implementation! I think you need to fix the mobile view left and right padding, so the elements dont just span across the page as it is now.
Marked as helpful0 - @aaron-romanickSubmitted over 3 years ago
How to Play
- You pick a "token" to see if you beat the house.
- Chances of winning are always 50%-50%; you will never tie.
- You can switch between "normal" mode and "bonus" mode by clicking logo image
- Both normal and bonus mode scores are saved in
localStorage
so they will persist on the next visit to the page
CSS
- While there were a lot of colors included in the style guide, I had to guess some of them from the design screenshots (eg: edge of tokens, shadows, focus glow).
- To accommodate CSS transitions, I have a lot of high specificity selectors, so I feel the CSS is kind of bloated, and any hints as to how to simplify it would be appreciated.
- Since there are a lot of transitions, I included the
will-change
property, but I've also read this can put on a strain on resources. I didn't notice a drop in performance either with or without it, so maybe it would be better to go without? - For the rules dialog on big screens, I wanted it to appear in the middle of the page regardless of window size, but I could only get it to work by setting the elements
top
property and animating it. I know animating thetop
property requires a re-paint of the page and thus is not high-performance. If anyone can think of a way to get the rules dialog exactly centered on the page without using it, please let me know. - I had to figure out what the coordinates of the position of each tokens around a circle was using trigonometry and putting each final value in by hand. Would there be a better way to figure it out with the
calc
function? - I'm using quite a few CSS variables, but I feel like I could have done more to make it more DRY. If you have any suggestions, I'd love to hear them.
- In Safari, the SVG images get really blurry when scaled; I looked around but couldn't find a solution that fixed it. Please let me know if there's anything I could have done!
HTML
- since this isn't some article/typical HTML and has lots of interaction/animations, I was kind of stuck as to how to organize the HTML. Is there a better way to lay this out?
@iadefidipePosted over 3 years agoThis is great, I love the animations. I am definitely studying your implementation.
0 - @peirstomSubmitted over 3 years ago
This was my first challenge on Frontend Mentor. It took me a full day to develop this. My major obstacle was to create responsive design. I took this challenge to confront myself with where I am standing currently with my design skills. I learned that I have a lot to learn to become more effective at writing SCSS. I am still have problems understanding the Flexbox properties, media queries and mixins.
@iadefidipePosted over 3 years agoGreat job Tom, you might want to work on your font sizes on smaller screens, other than that, you did a very nice job. Also check out the issues flagged by the platform. Keep Learning! cheers
Marked as helpful0 - @dewslyseSubmitted over 3 years ago
Comments and feedback welcomed. Thanks.
@iadefidipePosted over 3 years agoGreat job, your implementation is almost pixel perfect. The platform flagged some accessibility issues, you should check that out. cheers
0 - @zoknSubmitted over 3 years ago
It is not that complex, it comes in handy to reinforce css
@iadefidipePosted over 3 years agoGreat job.
-
Instead of using ID which should be unique for each element. Use classes, it makes your styling more easy to, and you can use a class for more than one element too
-
the whole card is not well centered on all screens.
-
you should have main container, so you can center the body element. You can also put the text and image section in the container and flex them. See if that helps.
0 -
- @krishna-nayakSubmitted over 3 years ago
I tried my best. Give your opinion or cool idea by which i can improve this page.
@iadefidipePosted over 3 years agoLooks, great on all screens. But since you have got a style sheet, you dont need the inline styles
1 - @YannikS14Submitted over 3 years ago
I am wondering if there is a better way to position the bubbles in the background. Currently it's positioned like in the design examples, but just for widths 1440px and 375px. If the width changes, the bubbles move unintended or out of the visible area.
@iadefidipePosted over 3 years agoYou can create media queries for more screen widths and style/position the background for them. Start the mobile background image from the 768px screen width and position it for that width, keep the mobile screen size position. you can position the desktop background Size for 1024px screen width too.
Your implementation is great, keep coding!
0