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

Boilerplate #3

Merged
merged 5 commits into from
May 4, 2020
Merged

Boilerplate #3

merged 5 commits into from
May 4, 2020

Conversation

BrandyMello
Copy link
Collaborator

@BrandyMello BrandyMello commented Apr 29, 2020

Fixes #1

  • Fixes or adds functionality/styling above without causing other issues.
    Explain functionality/styling:
    Adds the react-app boilerplate and dependancies, with icons for sharing and a (to be modified) logo. I want to add sass for styling, still.

  • Fixes or adds functionality above, but adds an issue.
    New Issue:

@Rafna

@BrandyMello BrandyMello requested a review from Rafna April 30, 2020 16:11
Copy link
Member

@jischr jischr left a comment

Choose a reason for hiding this comment

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

Looks good. a few small comments but they're mostly subjective.

import { FaFacebookMessenger } from 'react-icons/fa';
import Logo from '../../assets/plate.opt.svg';
import './App.css'
// import { render } from '@testing-library/react';
Copy link
Member

Choose a reason for hiding this comment

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

remove comment?

return (
<>
<header>
<img src={Logo} alt=''/>
Copy link
Member

Choose a reason for hiding this comment

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

add alt for logo for accessibility.

font-size: 4em;
position: relative;
top: 83px;
left: -164px;
Copy link
Member

Choose a reason for hiding this comment

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

love the look of the header :) I worry a bit about the specificity of the pixels but seems to be looking good on all the screen sizes.

}



Copy link
Member

Choose a reason for hiding this comment

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

small thing: but lots of extra lines here. 1 is good.


render() {
return (
<>
Copy link
Member

Choose a reason for hiding this comment

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

is this meant to be empty?

@jischr
Copy link
Member

jischr commented May 2, 2020

Ah also two console errors to fix:
Screen Shot 2020-05-01 at 8 13 00 PM

Copy link
Member

@Rafna Rafna left a comment

Choose a reason for hiding this comment

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

Did we fix the error?

@BrandyMello
Copy link
Collaborator Author

BrandyMello commented May 4, 2020 via email

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.

Create app boilerplate
3 participants