Closes #514 #555
Closes #514 #555
Conversation
There was a problem hiding this comment.
Very nice! Thank you so much!
In future it would be best to address different issues in different PRs, but these changes are all good.
Some things to consider for future iterations on these features:
- Ideally if a user selects a font and size, that should persist using localStorage, so that it's there when they refresh the page
- The connection status is good, but does not handle the case of switching from "Saving..." to "Saved", which is something I'd really like to see in a future iteration
Thanks again!
| const handleFontChange = (event) => { | ||
| let { body } = document; | ||
| body.style.setProperty( | ||
| '--vzcode-font-family', |
| onChange={handleFontChange} | ||
| > | ||
| {fonts.map((font) => ( | ||
| <option key={font} value={font}> |
There was a problem hiding this comment.
This does not show the previously selected option when you close the modal then re-open it.
There was a problem hiding this comment.
Indeed, it is correct! Now that the first version is ready, the next steps become clear.
I created a new issue to track the persistence: #562
There was a problem hiding this comment.
Oh but this comment was related to just the modal closing and re-opening, within a single session. I solved it in https://github.com/vizhub-core/vzcode/pull/559/files#diff-3cc8b6f15ff7e81275b96ef7bc6d64e13b89785525e651ccc6800b5da75aefa9R156
Using CSS variables for font family and the font size was easy to handle globally, and they were already present in the root, so no need to declare new variables and handle them differently, and also became handy for us to handle them throughout the app. We can add more fonts and more font sizes simply by loading them in the fonts.ts and font sizes array and we are good to go.
I have implemented #410 too, for detecting the connection status too.
We can close #514 and #410
Thanks in advance : )