-
-
Notifications
You must be signed in to change notification settings - Fork 15
feat: add html-eslint/parser explorer #99
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
Conversation
✅ Deploy Preview for eslint-code-explorer ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| }, | ||
| }, | ||
| optimizeDeps: { | ||
| include: ["@html-eslint/eslint-plugin"], |
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.
html-eslint is written in commonjs. (https://vite.dev/config/dep-optimization-options)
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.
Thanks for doing this great work👍
I’ve left a few comments about some missing parts.
src/components/ast/html-ast.tsx
Outdated
| <Accordion | ||
| type="multiple" | ||
| className="px-8 pb-4 font-mono space-y-3 min-w-max" | ||
| defaultValue={["0-StyleSheet"]} |
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.
| defaultValue={["0-StyleSheet"]} | |
| defaultValue={["0-Program"]} |
Oops!
| import { CssAst } from "./css-ast"; | ||
| import { MarkdownAst } from "./markdown-ast"; | ||
| import type { FC } from "react"; | ||
| import { HtmlAst } from "./html-ast"; |
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.
| import { CssAst } from "./css-ast"; | |
| import { MarkdownAst } from "./markdown-ast"; | |
| import type { FC } from "react"; | |
| import { HtmlAst } from "./html-ast"; | |
| import { CssAst } from "./css-ast"; | |
| import { MarkdownAst } from "./markdown-ast"; | |
| import { HtmlAst } from "./html-ast"; | |
| import type { FC } from "react"; |
This is a minor stylistic suggestion. grouping the ***Ast modules together would improve clarity.
Co-authored-by: Nicholas C. Zakas <nicholas@humanwhocodes.com>
Co-authored-by: Nicholas C. Zakas <nicholas@humanwhocodes.com>
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.
LGTM. Would like @lumirlumir to review before merging.
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.
LGTM, Thanks!
|
@lumirlumir when you're the last one to review and no one has requested other reviews, you can go ahead and merge PRs. 😄 |
|
@nzakas Thank you! I've merged it. |
Prerequisites checklist
What is the purpose of this pull request?
What changes did you make? (Give an overview)
Related Issues
Is there anything you'd like reviewers to focus on?
I downloaded the logo image from "https://www.w3.org/html/logo/".