42 - styling page footer - Man #55
Conversation
npm add react-range App.js - remove import DataFetchingTest from "./pages/DataFetchingTest";
passed data with props removed old code
removed hardcoded data
There was a problem hiding this comment.
Nice job with flex and matching the designs:
I'd like you to restructure some CSS and move content out of the code per my comments. This will make the component easier to use and update in the future.
Also, it looks like the vertical spacing is being thrown off by the default <h3> sizing in the OrganisationalLink component. This should be a small fix that results in a better match to the design.
|
|
||
| const ExternalLinks = ({ companyIcon, text, linkUrl }) => { | ||
| return ( | ||
| <a href={linkUrl}> |
There was a problem hiding this comment.
Let's do target="_blank" to open these in a new tab
There was a problem hiding this comment.
Same for OrganisationLink
client/src/components/Footer.css
Outdated
| @@ -0,0 +1,65 @@ | |||
| @import url('https://fonts.googleapis.com/css2?family=Roboto:wght@400;500;700&display=swap'); | |||
There was a problem hiding this comment.
Again it would be good to have this at a higher level so you don't need to import them in multiple places
client/src/components/Footer.js
Outdated
| <footer> | ||
| <div className="footer-disclaimer"> | ||
| <p> | ||
| Working under the artistic name Unity, Deirdre Molloy will exhibit the Black Atlantic Rhythm Codes map in Nantes, France in 2024. All contributors will be credited, with permission, including the creators of this Code Your Future prototype. The exhibition will be co-produced by Gerador and promoted by Project Manifest. |
There was a problem hiding this comment.
I recommend moving all content into a const content = {} object to separate copy from code.
E.g.
const content = {
text: "example"
};
const Component = () => <div>{content.text}</div>;This makes it easier to tell what's code and what's not.
client/src/components/Footer.js
Outdated
| </div> | ||
| <div className="flex-container"> | ||
| <div className="organisation-links-container"> | ||
| <OrganisationLink organisationName="decodenoir.org" organisationText="This interactive map is part an ethnography exhibition concept that aims to seed a coherent African Diaspora identity narrative in hearts and minds worldwide" organisationUrl="https://www.decodenoir.org/" /> |
There was a problem hiding this comment.
I'd recommend mapping through an array of links here
client/src/components/Footer.js
Outdated
| <div className="external-links-container"> | ||
| <div> | ||
| <p className="medium-font">Engineered by Team Rhythm Code</p> | ||
| <ExternalLink companyIcon={linkedinIcon} text="Christina Mifsud" linkUrl="https://github.com/christina-mifsud" /> |
There was a problem hiding this comment.
Again a good idea to map through an array
| return ( | ||
| <div className="organisation-container"> | ||
| <a href={organisationUrl}> | ||
| <h3 className="bold-font">{organisationName}</h3> |
There was a problem hiding this comment.
Looking at the designs vs the component, the font looks too big - the h3 is defaulting to 28px (1.75em) when it should be 20px (1.25em)
client/src/components/Footer.css
Outdated
| justify-content: space-between; | ||
| } | ||
|
|
||
| .link { |
There was a problem hiding this comment.
This css should be with the ExternalLink component - try creating an ExternalLink.css file and import it into ExternalLink.js
There was a problem hiding this comment.
Same with .link-icon and .bold-font which is only used in OrganisationalLink.
By housing the CSS with the relevant component, you make them more reusable. With this CSS setup, these components can only be used in the Footer, otherwise they break.
moved data into object set elements to use the object data instead
moved data into array set elements to use the array data instead
moved data into array set elements to use the array data instead
prev mapping of array created a component for project links which is separate
Naming Rules
Name your PR like this: ISSUENUMBER-TITLE-YOURNAME
Description
Related to
Make sure you include the issue number with a hash sign # so Github can link this PR to the right issue, like this:
Addresses #42
Checklist: