Prettier enhancements: handle more file types & improve loading states#184
Prettier enhancements: handle more file types & improve loading states#184VirtualDOMinic wants to merge 2 commits intomasterfrom
Conversation
- Add to prettier state (text and icons) - Extract prettier logic to util file - Set correct parder for a few common filetypes beyond js - Hide the prettier button if not able to process the file type
| setPrettierButtonState(afterPrettierState); | ||
| } catch (e) { | ||
| console.error(e); | ||
| setPrettierButtonState(errorPrettierState); |
There was a problem hiding this comment.
💭 put the button state setting in the UI component? better separation of concerns?
There was a problem hiding this comment.
💭 right after the call to loadPrettierParserScriptForExtension?
https://github.com/FormidableLabs/runpkg/pull/184/files#diff-bb8820440d0617732459bfe856f864abR42
There was a problem hiding this comment.
I'm 50/50 about this! On one hand it could be nice to keep the setStates in the component where the useState is used instead of passing the setter to this file, but on the other hand I feel this way keeps the Article component less cluttered (and is a bit easier).
Open to going the route you suggest though 🤔. Might see what others think
| const beforePrettierState = { | ||
| message: prettierButtonMessages.before, | ||
| disabled: false, // redundant? | ||
| icon: PrettierIcon, | ||
| }; | ||
|
|
||
| const duringPrettierState = { | ||
| message: prettierButtonMessages.during, | ||
| disabled: true, | ||
| icon: LoadingIcon, | ||
| }; | ||
|
|
||
| const afterPrettierState = { | ||
| message: prettierButtonMessages.after, | ||
| disabled: true, | ||
| icon: CheckMarkIcon, | ||
| }; | ||
|
|
||
| const errorPrettierState = { | ||
| message: prettierButtonMessages.error, | ||
| disabled: true, | ||
| icon: ErrorIcon, | ||
| }; | ||
|
|
||
| const cannotPrettierState = { | ||
| hidden: true, | ||
| icon: null, | ||
| }; |
There was a problem hiding this comment.
🤗 Yeah I prefer the neatness of having these separated out clearly! Probably influenced by TS enums and Redux
|
@lukejacksonn could you close this PR/ remove me as a reviewer please 😊 |
3 participants
Covers issue #183
Also relates to issue #176
Enhancements in this PR:
ts,css,json)See comments left inline, too!