-
Notifications
You must be signed in to change notification settings - Fork 189
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
issue-2423: Added state to store student quote #2510
Conversation
c388d19
to
fb8a792
Compare
src/web/src/components/Banner.tsx
Outdated
export default function Banner({ onVisibilityChange }: BannerProps) { | ||
const classes = useStyles(); | ||
const [gitInfo, setGitInfo] = useState({ | ||
gitHubUrl: '', | ||
sha: '', | ||
version: '', | ||
}); | ||
const studentQuote = getQuote(); | ||
const [studentQuote] = useState(() => quotes[Math.floor(Math.random() * quotes.length)]); |
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.
Let's consider doing what @DukeManh suggested and move this out of the component. Put it on line 113 above and do:
const studentQuote = quotes[Math.floor(Math.random() * quotes.length)]);
Then you can use it in the component, but it shouldn't change its value on re-render.
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.
Perfect. I tried the solution but didn't quite understand we had to move it out, and it had not worked. This works now! Thank you.
removed states, and kept const for student quotes
48560a8
to
29a5431
Compare
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.
👍
Issue This PR Addresses
fixes #2423
Type of Change
This PR adds the sets a state for the student quote, so that when you scroll on the page, it doesn't grab a new quote to display.
Description
Checklist