-
Notifications
You must be signed in to change notification settings - Fork 248
Maintain font size in grading view #7525
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
base: master
Are you sure you want to change the base?
Maintain font size in grading view #7525
Conversation
…est test regarding saving font size to localStorage
Currently, I am working on a Jest test to see if when we visit the grading view, if the font size variable in localstorage is being used to set the state, font_size. |
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.
@donny-wong good work, I just left a few comments.
Regarding the remaining test cases, I suggest using a jest mock to return a particular value for the local storage when accessing text_viewer_font_size
. See e.g. https://jestjs.io/docs/mock-functions.
@@ -37,6 +37,10 @@ export class TextViewer extends React.PureComponent { | |||
componentDidMount() { | |||
this.highlight_root = this.raw_content.current.parentNode; | |||
|
|||
if (localStorage.getItem("text_viewer_font_size") != null) { |
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.
in general, use triple-equals instead of double-equals in Javascript.
@@ -37,6 +37,10 @@ export class TextViewer extends React.PureComponent { | |||
componentDidMount() { | |||
this.highlight_root = this.raw_content.current.parentNode; | |||
|
|||
if (localStorage.getItem("text_viewer_font_size") != null) { | |||
this.setState({font_size: Number(localStorage.getItem("text_viewer_font_size"))}); |
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 do the Number parsing in a try-catch; if the number fails to parse, don't set state and instead delete the localStorage value
Proposed Changes
When the font size is modified by the user in the grading view for the editor, we want to maintain that size even when the browser is refreshed. To do this we store the font size in the browser's local storage, where we check for and apply it when the browser is refreshed.
...
Screenshots of your changes (if applicable)
This image shows where the editor's font size buttons are located:Associated documentation repository pull request (if applicable)
Type of Change
(Write an
X
or a brief description next to the type or types that best describe your changes.)Checklist
(Complete each of the following items for your pull request. Indicate that you have completed an item by changing the
[ ]
into a[x]
in the raw text, or by clicking on the checkbox in the rendered description on GitHub.)Before opening your pull request:
After opening your pull request:
Questions and Comments
(Include any questions or comments you have regarding your changes.)