CUJ index [land silently] - #13769
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.
Adds the canonical Critical User Journeys (CUJs) data, data model, filterable index page components, and styling for /ai/flutter-bench/cujs.
|
Staged preview of the updated docs.flutter.dev site (updated for commit b4619e5): https://flutter-docs-prod--docs-pr13769-cuj-index-ucwmiabz.web.app |
|
Staged preview of the updated flutter.dev site (updated for commit b4619e5): https://flutter-dev-230821--www-pr13769-cuj-index-eerbuymc.web.app |
# Conflicts: # sites/docs/lib/_sass/components/_filterable-index.scss # sites/docs/lib/src/components/pages/filterable_index.dart # sites/docs/lib/src/components/pages/learning_resource_filters_sidebar.dart
There was a problem hiding this comment.
Code Review
This pull request introduces a new Critical User Journey (CUJ) index page, including a catalog of journeys, search and filter capabilities, and responsive styling. The feedback highlights several critical syntax errors where .text(...) is used instead of Text(...), which will cause compilation failures. Additionally, the reviewer pointed out potential runtime crashes related to unsafe type casting of page data and checkbox states, a potential exception from calling setState on an unmounted widget, and recommended using a more specific CSS selector for querying card elements.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
lamek
left a comment
There was a problem hiding this comment.
LGTM on the docs stuff. I added some fixes and suggestions. I'll leave the code review to Parker and Killian.
Page I reviewed: https://flutter-docs-prod--docs-pr13769-cuj-index-ucwmiabz.web.app/ai/flutter-bench/cujs
| concrete tasks they take to get there. | ||
|
|
||
| Product teams at Google treat CUJs as a source of truth: they're how teams align | ||
| on priorities, shape roadmaps, measure product health and more. And we use |
There was a problem hiding this comment.
Add comma: "...manage health, and more."
| concrete tasks they take to get there. | ||
|
|
||
| Product teams at Google treat CUJs as a source of truth: they're how teams align | ||
| on priorities, shape roadmaps, measure product health and more. And we use |
There was a problem hiding this comment.
Remove the leading "And" --> "The Flutter team uses CUJs..."
|
|
||
| The catalog below is the Flutter team's CUJs. It's a claim about what matters in | ||
| Flutter development. If the way you build Flutter apps isn't represented here, | ||
| the list is incomplete, and we encourage [your feedback][]. |
There was a problem hiding this comment.
"... you to open an issue with your feedback."
Suggest adding this to make it clear what the user should do once they land on the issues page.
|
|
||
| [FlutterBench]: /ai/flutter-bench | ||
|
|
||
| [critical user journey]: https://static.googleusercontent.com/media/sre.google/en//static/pdf/art-of-slos-handbook-a4.pdf |
There was a problem hiding this comment.
Is this the correct thing to link to? I see the section on user journeys, but the PDF is mostly about SLOs.
There was a problem hiding this comment.
ah shoot that isn't right, I meant to return to this
| shortTitle: Flutter CUJs | ||
| description: > | ||
| Browse the catalog of canonical Flutter and Dart critical user journeys | ||
| (CUJs). FlutterBench evals measures these CUJs. |
There was a problem hiding this comment.
measures --> measure (remove the "s")
| on priorities, shape roadmaps, measure product health and more. And we use | ||
| CUJs to derive evaluation tasks and prompts for [FlutterBench][]. | ||
|
|
||
| The catalog below is the Flutter team's CUJs. It's a claim about what matters in |
There was a problem hiding this comment.
Suggest changing "is the" to "lists"
"The catalog below lists the Flutter..."
|
|
||
| A _[critical user journey][]_ (CUJ) is a goal a developer sets out to | ||
| accomplish, like "make an application accessible to all users" or | ||
| "diagnose and resolve layout overflow errors" and it's broken down into the |
There was a problem hiding this comment.
Add a comma after "errors"
...layout overflow errors", and it's...
sfshaza2
left a comment
There was a problem hiding this comment.
I'm a bit confused here. I looked at the staged site and I can't find the /ai/flutter-bench page that is referred to in the PR. I know that @jesskuras's restructuring PR does have that page, so is this PR waiting for her PR to land? If so, maybe mark this as draft...
The path will change depending on Jess's change's, so I'm just waiting for that PR to land to know what to update it to. |
parlough
left a comment
There was a problem hiding this comment.
Cool! Generally looking good to me. Some small suggestions:
| description: > | ||
| Browse the catalog of canonical Flutter and Dart critical user journeys | ||
| (CUJs). FlutterBench evals measures these CUJs. |
There was a problem hiding this comment.
Consider tightening this to a single sentence:
| description: > | |
| Browse the catalog of canonical Flutter and Dart critical user journeys | |
| (CUJs). FlutterBench evals measures these CUJs. | |
| description: >- | |
| Browse the catalog of canonical Flutter and Dart critical user journeys that | |
| the FlutterBench evaluations test. |
| on priorities, shape roadmaps, measure product health and more. And we use | ||
| CUJs to derive evaluation tasks and prompts for [FlutterBench][]. | ||
|
|
||
| The catalog below is the Flutter team's CUJs. It's a claim about what matters in |
There was a problem hiding this comment.
Change "The catalog below" to "The following catalog".
| A _[critical user journey][]_ (CUJ) is a goal a developer sets out to | ||
| accomplish, like "make an application accessible to all users" or | ||
| "diagnose and resolve layout overflow errors" and it's broken down into the | ||
| concrete tasks they take to get there. |
There was a problem hiding this comment.
This is a long sentence. Consider breaking it up into two. Perhaps something like:
A _[critical user journey][]_ (CUJ) is a goal that
a developer sets out to accomplish,
such as "make an application accessible to all users" or
"diagnose and resolve layout overflow errors".
Each CUJ is broken down into the concrete tasks required to complete it.
|
|
||
| <CujIndex /> | ||
|
|
||
| [FlutterBench]: /ai/flutter-bench |
There was a problem hiding this comment.
Since this page doesn't exist yet, consider dropping the cross-link or leave a basic index page in its place saying coming soon, linking to the GitHub, or linking to this page.
| to render rich vector animations within the application. | ||
| - id: 57 | ||
| name: use-shaders-animate-things-app | ||
| task: Implement FragmentShaders using GLSL shader programs to render custom |
There was a problem hiding this comment.
Since there's no exact type FragmentShaders, consider changing this to "fragment shaders":
| task: Implement FragmentShaders using GLSL shader programs to render custom | |
| task: Implement fragment shaders using GLSL shader programs to render custom |
If you do want to refer to the type, you could say "FragmentShader objects":
task: Implement FragmentShader objects using GLSL shader programs to render custom
| @@ -0,0 +1,1040 @@ | |||
| - id: 0 | |||
There was a problem hiding this comment.
Are these IDs going to be stable? Like if this task is removed, a new one won't take this ID? They're used for the links to the cards, so I wanted to be sure.
| div(classes: 'cuj-feedback', [ | ||
| Button( | ||
| href: _feedbackUrl, | ||
| content: 'Feedback', |
There was a problem hiding this comment.
Consider changing this to "Provide feedback" to be action oriented.
| content: 'Feedback', | |
| content: 'Provide feedback', |
| href: _feedbackUrl, | ||
| content: 'Feedback', | ||
| style: ButtonStyle.outlined, | ||
| title: 'Leave feedback or suggest new CUJs', |
There was a problem hiding this comment.
| title: 'Leave feedback or suggest new CUJs', | |
| title: 'Leave feedback or suggest new CUJs.', |
There was a problem hiding this comment.
Is this coming from elsewhere or is this the source of truth?
If it's coming from elsewhere, please add a comment to the top indicating from where or how its generated.
If this is the source of truth, add a comment explaining the expected structure. https://github.com/flutter/website/blob/main/sites/www/content/blog/authors.yaml is a basic example.
| .expand-button { | ||
| &:hover, | ||
| &:focus-within { | ||
| transition: transform $transition-normal; | ||
| } | ||
| } |
There was a problem hiding this comment.
No need to conditionally apply the transition. It'll only apply when transitioning.
| .expand-button { | |
| &:hover, | |
| &:focus-within { | |
| transition: transform $transition-normal; | |
| } | |
| } | |
| .expand-button { | |
| transition: transform $transition-normal; | |
| @media (prefers-reduced-motion: reduce) { | |
| transition: none; | |
| } | |
| } |
Description of what this PR is changing or adding, and why:
This PR adds a CUJ index page, which is landed silently for now. This is an effort to break up the currently massive #13691