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

✨ Pretty Printing HTML in console.logs. #167

Merged
merged 1 commit into from Oct 30, 2019

Conversation

rahulakrishna
Copy link
Contributor

@rahulakrishna rahulakrishna commented Oct 28, 2019

Use pre tag to properly format HTML
🚨 Add index as key to map inside console-panel/index.tsx. This is to get rid of the error message in the console. Since we don't have any identifier for each log. React anyway defaults to using index as key when not explicitly given a key.

Often times when working with tests I have to do console.log(component.debug() which outputs a prettified html code onto the console. Majestic however shows the whole component in a single line which can be hard to debug.

Before:
image

After:
image

@Raathigesh
Copy link
Owner

Hey, Thaks for the PR.

Could you please share how does the prettier make things better? A real example of before and after would be nice.

And if it makes things better, we should try to consume it via npm rather than linking it via CDN :) Is there a particular reason for linking it via CDN?

@rahulakrishna
Copy link
Contributor Author

HI, I have updated the PR description with before and after images.

The package I'm using is code-prettify from Google. Their instructions only show how to set-up using a CDN. I found an attempt here (react-code-prettify) at making it consumable via npm package. But react-code-prettify seems to be pretty limited (with the last commit coming at 2017 and no support for HTML) right now.

@rahulakrishna
Copy link
Contributor Author

rahulakrishna commented Oct 28, 2019

Changed the after screenshot changing log fonts to monospace

@Raathigesh
Copy link
Owner

Hi again,

Thanks for posing the images. It looks like it would be useful to have the console log pretty-printed if we are console logging Html. However, I would still like to go with an npm based solution.

We could try the standalone prettier version if you are interested. https://prettier.io/docs/en/browser.html#commonjs

@rahulakrishna
Copy link
Contributor Author

Hey @Raathigesh , Good that you insisted! Apparently we don't need an external library to format HTML code. Wrapping it in <pre> works well enough. Gonna revert some of my changes

💄 Uses `pre` to prettify logs in `console-panel/index.tsx`
💄 Change Log fonts to monospace
🚨 Add index as a key to `map` function in `console-panel/index`
@rahulakrishna rahulakrishna changed the title ✨ Pretty Printing console.logs. ✨ Pretty Printing HTML in console.logs. Oct 29, 2019
@rahulakrishna
Copy link
Contributor Author

@Raathigesh I am also looking into leveraging react-inspector to render DOM Elements. Do you want that in this PR or a separate PR?

@rahulakrishna
Copy link
Contributor Author

Hey, Can we merge this?

@Raathigesh
Copy link
Owner

Thanks for the Update. Let's make the react-inspector change in a separate PR. Just a quick question. How does the pre-change look for non-code values?

@rahulakrishna
Copy link
Contributor Author

image

Everything else looks fine. The existing logic passes every type other than string to the ObjectInspector. So only strings are wrapped in pre.

@rahulakrishna
Copy link
Contributor Author

I assume this PR resolves #163 If I understand that correctly

@Raathigesh Raathigesh merged commit c5fdd92 into Raathigesh:master Oct 30, 2019
@Raathigesh
Copy link
Owner

Ah Right. Looks good to me.

@allcontributors please add @rahulakrishna for code and idea

@allcontributors
Copy link
Contributor

@Raathigesh

I've put up a pull request to add @rahulakrishna! 🎉

@Raathigesh
Copy link
Owner

Thank you for your PR. I will push a release by this week.

@CurtisHumphrey
Copy link

@rahulakrishna did you start the PR to use react-inspector as a visualizer for DOM output?

@rahulakrishna
Copy link
Contributor Author

Haven't gotten around to it yet.

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.

None yet

3 participants