Conversation
… removed containers in FAQ
…enerated by map(), add click handler to past menu carousel
| @@ -13,3 +13,15 @@ Open [http://localhost:3000](http://localhost:3000) to view it in your browser. | |||
|
|
|||
There was a problem hiding this comment.
Please also include how to install your dependencies to get the project going
| The page will reload when you make changes.\ | ||
| You may also see any lint errors in the console. | ||
|
|
||
| ### User stories |
There was a problem hiding this comment.
really nice user stories and wireframe
| } | ||
|
|
||
| .about { | ||
| padding-left: 3%; |
There was a problem hiding this comment.
what is 3% padding? I think using percentages here can be confusing. I would rather use pixels
| if (window.location.href.includes("#")) { | ||
| const urlList = window.location.href.split("#"); | ||
| const id = urlList[urlList.length - 1]; | ||
|
|
||
| document.querySelector(`#${id}`).scrollIntoView(); | ||
| } | ||
| }; | ||
|
|
||
| moveTo = (id) => { | ||
| document.querySelector(`#${id}`).scrollIntoView(); | ||
| }; |
There was a problem hiding this comment.
Nice hack you found here. However, for react, we don't want to directly select the DOM elements. Since we work with a shadow DOM, we want to work on that one instead. To do that, we can use a reference, short Ref
https://stackoverflow.com/questions/43441856/how-to-scroll-to-an-element
| <p>8 to 11 (Wed to Sat)</p> | ||
| <p>15 to 18 (Wed to Sat)</p> | ||
| <p>22 to 25 (Wed to Sat)</p> |
There was a problem hiding this comment.
Hardcoding this is fine for now. Later it would be good to make this dynamic! Maybe have a calendar of sorts, and print the Wed to Saturday dates there for whichever months you are listing here. Maybe you could also make it the whole year with a collapsible to show everything beyond the current 2 months? You can probably achieve this with the library datefns or moment :)
| render() { | ||
| return ( | ||
| <div className="row justify-content-center"> | ||
| <h1>{this.props.menu}</h1> |
There was a problem hiding this comment.
Since menu can be null, you might render an empty h1 here. I would conditionally render this
| <div className="row justify-content-center"> | ||
| <h1>{this.props.menu}</h1> | ||
| <p className="col-lg-10 col-12"> | ||
| {this.pastMenuDict[this.props.menu].description} |
There was a problem hiding this comment.
Since menu can be null, you might access .description of undefined here, which would break your app
| plate for all to indulge in. | ||
| </p> | ||
| <Carousel interval={null} className="col-lg-8 col-12"> | ||
| {this.pastMenuDict[this.props.menu].starters.map((starter) => ( |
| <h1 className="section-header">Dessert</h1> | ||
| <p className="col-lg-10 col-12"> | ||
| There is only one dessert choice to round of the meal. | ||
| </p> | ||
| <div | ||
| className="past-menu car-item dessert-container col-lg-8 col-12" | ||
| style={{ | ||
| backgroundImage: `url(${ | ||
| this.pastMenuDict[this.props.menu].desserts[0].image | ||
| })`, | ||
| }} | ||
| > | ||
| <div className="past-menu-caption dessert-caption"> | ||
| <h3>{this.pastMenuDict[this.props.menu].desserts[0].title}</h3> | ||
| <p> | ||
| {this.pastMenuDict[this.props.menu].desserts[0].description} | ||
| </p> | ||
| </div> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
Would be nice to create separate components for this, as it would make more sense when reading it, as to what it is.
| <h2 className="book-header"> | ||
| Scan the code below or click to stay updated with our intimate | ||
| events | ||
| </h2> | ||
| <a href="https://t.me/+51G0to4dOdM4YTBl"> | ||
| <img | ||
| className="col-lg-2 col-sm-3 col-3" | ||
| src={teleQR} | ||
| href="https://t.me/+51G0to4dOdM4YTBl" | ||
| alt="QR code to join Telegram channel" | ||
| /> | ||
| </a> | ||
| </div> | ||
| <div className="follow"> | ||
| <Follow /> | ||
| </div> |
There was a problem hiding this comment.
This could also be an own component. Anytime you see sections that have a specific responsibility and are possibly re-used, creating a component would make sense
| <Navbar.Toggle aria-controls="basic-navbar-nav" /> | ||
| <NavbarCollapse id="basic-navbar-nav"> | ||
| <Nav> | ||
| <Link |
There was a problem hiding this comment.
We could define a data structure for these links.
const links = [
{
to: "your link",
className: "class"
onClick: () => { do stuff }
}
]
2 participants
Patrick's pull request for Rocket Academy Project 1