prefactor: generalize filterable index layout and components - #13768
Conversation
Extract generic two-column filterable index layout styles and sidebar/search components from learning resources into reusable filterable_index.dart and _filterable-index.scss.
There was a problem hiding this comment.
Code Review
This pull request refactors the filterable index pages by extracting shared layout, sidebar, and search group components and styles into reusable Dart components (FiltersSidebar, FilterSearchGroup) and a shared SASS stylesheet (_filterable-index.scss), successfully reducing code duplication. The feedback highlights a critical issue in filterable_index.dart where event.target is unsafely cast to web.Element? using the 'as' operator, which can lead to a runtime TypeError. It is recommended to use a safe type check (is web.Element) instead.
|
Staged preview of the updated docs.flutter.dev site (updated for commit 1263a08): https://flutter-docs-prod--docs-pr13768-refactor-filterable-in-fnqqj8yi.web.app |
|
Staged preview of the updated flutter.dev site (updated for commit 1263a08): https://flutter-dev-230821--www-pr13768-refactor-filterable-in-6oidcs4i.web.app |
parlough
left a comment
There was a problem hiding this comment.
Thanks for extracting this out @ericwindmill! Mostly looks good to me.
Should the learning resources index be updated to use the new FilterSearchGroup?
…or-filterable-index
Co-authored-by: Parker Lougheed <parlough@gmail.com>
…r/website into refactor-filterable-index
…or-filterable-index
parlough
left a comment
There was a problem hiding this comment.
Thanks for those updates @ericwindmill! Looks good to me after a few final adjustments:
Co-authored-by: Parker Lougheed <parlough@gmail.com>
Co-authored-by: Parker Lougheed <parlough@gmail.com>
Description of what this PR is changing or adding, and why:
Extract generic two-column filterable index layout styles and sidebar/search components from learning resources index into reusable filterable_index.dart and _filterable-index.scss.
This change anticipates new content for FlutterBench, where I'm adding a CUJ index #13691