-
Notifications
You must be signed in to change notification settings - Fork 104
#159 Adds Console Component for rendering of output #277
#159 Adds Console Component for rendering of output #277
Conversation
Looks cool! Ping me when you want me to review. |
I'll review asap |
|
||
useEffect(() => { | ||
// eslint-disable-next-line arrow-parens | ||
Hook(window.console, log => { |
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.
Hi @dafrie,
Here's a first suggestion:
Instead of using a hook on console.log, I suggest using a prop or expose a logger
class that is hooked to
https://github.com/tmrowco/tmrowapp-contrib/blob/master/playground/server/index.js#L77-L81
That will make the interface cleaner and more robust.
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.
Yep makes sense! I will go the props way. Expanding the main app state was actually my first shot at it, but then decided to have only within the console component - Did not look careful enough at the runLogs
listener!
is it possible to sort ASC by default and show all output by default (i.e. warn and debug by default?) |
Like so? In the ASC case however, then probably it should always scroll to bottom, so the whole console output should be wrapped in a scrollable container? |
This looks good for now! I suggest we merge and then we can make additional UX changes later on. |
Uses console-feed for rendering of output and allows searching, filtering and ordering of output. Resolves #159
TODO: