-
Notifications
You must be signed in to change notification settings - Fork 1
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
Component center img #143
Component center img #143
Changes from 17 commits
682bd19
4c9d413
ab1e697
d8610c2
805f4f2
5c826fb
2073dbe
efb389a
e464c2d
9f24607
4e7584c
87bd2aa
040fb9d
68f7be4
261b175
0fa61fe
e8a5908
3639294
aa91bf4
27b91da
8b899d3
2ceddef
834e37b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,26 @@ | ||||||
import React from "react"; | ||||||
|
||||||
/** | ||||||
* Component for centering an image in the docs | ||||||
* | ||||||
* @param {*} props : | ||||||
* src: image source | ||||||
* alt: image alternative text | ||||||
* imgStyle: object, any image styles (props can be passed in as: `imgStyle={{ maxHeight: "600px", border: "solid" }}`) | ||||||
* @returns a component for uniformally centering images in the doc | ||||||
*/ | ||||||
|
||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's an extra empty line here
Suggested change
|
||||||
export default function CenterImg(props) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is an example of how you could set up the component with prop delegation
Suggested change
|
||||||
let imgStyleString; | ||||||
if (props.imgStyle != undefined && Object.keys(props.imgStyle).length === 0) { | ||||||
imgStyleString = {}; | ||||||
} else { | ||||||
imgStyleString = props.imgStyle; | ||||||
} | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A great way of doing prop delegation is by using the spread operator There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see! This looks really nice and makes more sense to keep things clean. I tried to modify the current |
||||||
|
||||||
return ( | ||||||
<div style={{ textAlign: "center" }}> | ||||||
<img src={props.src} alt={props.alt} style={imgStyleString} /> | ||||||
</div> | ||||||
); | ||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple semantic things here
CenteredImage
as component are things ("A centered image") and not an action ("a function that centers an image") - hope that makes sense!CenteredImage.jsx
? Different companies may have different naming conventions but it's generally best to give the file the same exact name as the componentThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see! This makes so much sense; thank you so much :) ! Just went in and refactored the semantics.