Skip to content

Acrobatics & Crossfit sketch#42

Open
akipeki wants to merge 11 commits intoTechnigo:masterfrom
akipeki:master
Open

Acrobatics & Crossfit sketch#42
akipeki wants to merge 11 commits intoTechnigo:masterfrom
akipeki:master

Conversation

@akipeki
Copy link

@akipeki akipeki commented Apr 10, 2023

No description provided.

Copy link

@AnnikaSonnek AnnikaSonnek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nicely done!
I don't have the design that you got so I couldn't compare it to that one but I think it was easy to follow the code. I think there is one component (Button.js) that is not used anywhere (I could be wrong!).

Nicely done!

import './menu.css';
import Logo from './Logo';

const Menu = () => {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nicely done with the mobile menu! I wasn't able to make it display as normal when not in mobile view so I did another menu that I didn't display in mobile view and vice versa.

}

/* tablet rules - starts here */
@media (min-width: 391px) and (max-width: 1023px) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've never seen min-width and max-width being mentioned in a media query before, looks more clear doing it this way!

align-items: center;
background-color: #333366;
padding: 3em 0 3em 0;
--bs-gutter-x: 1.6rem;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this? I've never seen it before! What does it do?

onChange={handleChange}
value="subscribe" />
<br />
<input id="submitbutton" type="submit" value="Submit" />

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried this one out but nothing happened when I pressed the button, maybe you'll have to move the "onSubmit" to the button? Or create an OnClick for the button.

@@ -0,0 +1,17 @@
import styled, { css } from 'styled-components'

const Button = styled.button`

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I this component one used anywhere in the code?

import Slider from 'react-slick';
import './simple-slider.css';

const SimpleSlider = () => {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice that you got this react-slick slider to work! I struggled a lot with this one? Especially since something in the package made it incredibly big so it wouldn't display properly, did you have the same issue?

}
},
{
breakpoint: 600,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these sort of like mediaqueries? The breakpoints, I mean

@@ -0,0 +1,144 @@
/* responsive rules - starts here */

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea to keep all the responsiveness-css in one file. If you want to use styled components you can put the mediaqueries for each of those in the same place. So you would have the basic styling at the top and then directly below inside the backticks, you can have the media queries for that component.

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.

2 participants