Add an "ide" component - #13708
Conversation
|
Staged preview of the updated flutter.dev site (updated for commit 147c225): https://flutter-dev-230821--www-pr13708-ide-component-djgv8rr8.web.app |
|
Staged preview of the updated docs.flutter.dev site (updated for commit f8bfaa9): https://flutter-docs-prod--docs-pr13708-ide-component-1g9w9f0b.web.app |
There was a problem hiding this comment.
Code Review
This pull request introduces a new interactive file-tree explorer component (IdeExplorer) for documentation sites, including its SASS styles, Jaspr Dart components, a custom markdown parser, and client-side interactivity scripts. The feedback highlights several critical issues: a potential hydration mismatch in SSR due to a global static counter, invalid HTML nesting of a inside a
| (reasoning, safety, and conciseness) using automated model judges | ||
| and expert human grading. | ||
|
|
||
| {% render "docs/ai/flutter_bench_task_explorer.md" %} |
There was a problem hiding this comment.
This is for demo purposes, and needs to be removed before merged
There was a problem hiding this comment.
This is super cool @ericwindmill! I haven't completed a full review, but thought I'd leave some initial comments. Feel free to push back on anything.
Thanks!
| } | ||
|
|
||
| /// A single file or folder entry in an [IdeExplorer] tree. | ||
| class IdeTreeNode { |
There was a problem hiding this comment.
I'd consider making this a sealed class hierarchy and adding two children classes, one for files and one for directories. Since it seems a few of the members are specific to a specific node type.
|
|
||
| /// Set up interactivity of the file/detail explorer created with | ||
| /// the `<IdeExplorer>` custom component. | ||
| void _setUpIdeExplorers() { |
There was a problem hiding this comment.
If it's possible, I'd prefer to avoid adding this much new imperative DOM logic. While Jaspr support for server components is still in-progress, we have a ComponentRef type that can be used for now to store and reference the rendered Markdown content.
Using that, the IdeExplorer might be able to become a client component that accepts Map<String, ComponentRef> instead of Map<String, Component> for customContents.
Worth trying, but if it ends up being too complicated or messy, don't worry about it. We can always revisit it once server components are finalized.
There was a problem hiding this comment.
If you end up keeping this imperative logic, review it and if needed, update it to ensure the aria states on the elements are updated as appropriate.
Description of what this PR is changing or adding, and why:
This is adding an interactive IDE component, because its cool. I'm using it for the FlutterBench updates, but want to land this separately to keep the PR sane.
Depends on #13766