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

issue-2423: Added state to store student quote #2510

Merged
merged 1 commit into from
Nov 22, 2021

Conversation

AmasiaNalbandian
Copy link
Contributor

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.

  • Bugfix: Change which fixes an issue
  • New Feature: Change which adds functionality
  • Documentation Update: Change which improves documentation
  • UI: Change which improves UI

Description

Checklist

  • Quality: This PR builds and passes our npm test and works locally
  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not (if applicable)
  • Documentation: This PR includes updated/added documentation to user exposed functionality or configuration variables are added/changed or an explanation of why it does not(if applicable)

@AmasiaNalbandian AmasiaNalbandian self-assigned this Nov 22, 2021
@gitpod-io
Copy link

gitpod-io bot commented Nov 22, 2021

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)]);
Copy link
Contributor

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.

Copy link
Contributor Author

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
Copy link
Contributor

@humphd humphd left a comment

Choose a reason for hiding this comment

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

👍

@humphd humphd requested a review from DukeManh November 22, 2021 20:01
@DukeManh DukeManh merged commit c32d7ba into Seneca-CDOT:master Nov 22, 2021
@AmasiaNalbandian AmasiaNalbandian deleted the issue-2423 branch November 23, 2021 00:57
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.

Student Quote changes on scroll
3 participants