feat: add scan and create-custom actions to no-results search state (#578) - #995
feat: add scan and create-custom actions to no-results search state (#578)#995ibrahim-iqbal wants to merge 4 commits into
Conversation
…ing them (#1045) Codex reviews pull requests vendor-side and reads AGENTS.md to do it, so the conventions no generic reviewer could know now live in a Code Review Rules section near the top of the file. The rules are shaped by what a hand-run review of #995 got right and wrong. The one check CI cannot do is a key present in every locale with the English text still in it: check_l10n fails only on *missing* keys, so that ships green. Two rules exist purely to prevent confident false findings — ARB key counts legitimately differ between any two locales because every file carries its own @key metadata, and `container: true` is conditional on a layout-greedy parent, so its absence under a Wrap is correct. Codex truncates instruction files at 32 KiB, keeping the head and dropping the tail, with no warning anywhere a human looks. AGENTS.md was at 90.6% of that. Two blocks no reviewer can act on move out to make room: the ADB driver scripts and uiautomator gotchas to tools/adb/README.md, and the demo seeder explanation to docs/demo-data.md. Both keep a pointer. check_agents_md then fails the build before the cap is reached, and asserts the rules stay near the head where truncation cannot take them. Pure bash, so it runs first in linux-checks and needs no toolchain. Also groups the four weekly Dependabot ecosystems, which previously fanned out into a burst of individual bump PRs, and stops the PR template asking contributors to edit gitignored lib/generated/. AGENTS.md: 29,681 -> 28,140 bytes.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 431e41fb60
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| final state = locator<AddMealBloc>().state; | ||
| final imperial = state is AddMealLoadedState | ||
| ? state.usesImperialUnits | ||
| : false; |
There was a problem hiding this comment.
For users who selected imperial food units, this lookup creates a fresh AddMealBloc because that type is registered as a factory; the new instance has never received InitializeAddMealEvent, so the state is always initial and imperial always falls back to false. Consequently, creating a custom food from any new empty-state button opens the editor in metric mode, unlike the existing app-bar action, which reads its initialized bloc instance. Retain and read the screen's initialized bloc or pass the unit setting already available in the result state.
AGENTS.md reference: AGENTS.md:L355-L360
Useful? React with 👍 / 👎.
| return NoResultsWidget( | ||
| onScanBarcode: _onBarcodeIconPressed, | ||
| onCreateCustomFood: () => | ||
| _onCustomAddFromEmptyState(), | ||
| ); |
There was a problem hiding this comment.
When a remote source returns no rows or fails but matching custom, recipe, history, or cached meals exist, remoteSourceEmpty appends this widget after those visible matches. Passing the callbacks now makes that footer say “No foods matched that search” and display the full recovery actions, directly contradicting the results immediately above it; this path needs a distinct “no remote results” footer or should omit the new full-empty-state content.
Useful? React with 👍 / 👎.
| return Column( | ||
| mainAxisSize: MainAxisSize.min, |
There was a problem hiding this comment.
On a landscape phone or with a sufficiently large system text scale, the search header, source chips, and results header leave less vertical space than the fixed 72-pixel icon, padding, subtitle, and newly added action buttons require. This non-scrollable Column cannot shrink inside the tightly constrained results area, so it produces a RenderFlex overflow and can push the recovery buttons off-screen; allow the empty state to scroll or otherwise adapt to the available height.
Useful? React with 👍 / 👎.
|
Thanks for this — the empty state is a real gap and the two actions are the right ones. A few things to fix before it can go in. The localization is the blocker, and it is mostly copy-pasteThe PR adds three keys, and all three carry the English string in every one of the eight non-English ARBs. The good news: the app already says one of these.
For the other two, these follow the terminology already in the ARBs —
(Machine translation is an accepted starting point here per Three code pointsThe unit lookup returns the wrong bloc. The footer contradicts the results above it. At the Same on the Recent tab. With an empty query and no history the actioned widget now claims a search returned nothing when no search ran. Two smaller onesThe And a naming inconsistency worth catching now: the new button says "Create custom food" while the dialog it opens ( The |
|
Addressed all three points in
Existing widget tests still pass. |
|
Thanks for the thorough pass — everything's in as of Localization — all three keys now carry a real string in every locale. Recent tab — empty query + no history now renders Unit lookup / footer / scroll — these landed in Naming — I kept "Create custom food" because it matches the ARB majority ( |
Both come from findings that were verified against the code after an automated review raised them, on #995 and #1010. Attribution. On #1010 half the findings were true about the code and aimed at the wrong person: the reviewer did not separate what that PR introduced from prose it merely inherited, and asked a contributor to fix a sentence they had not touched. On a documentation-correction PR that distinction is the whole value of the review. Precedent. #995 added a key meaning "Scan barcode" and left it English in eight locales. The useful finding was not that it needed translating: customMealBarcodeScanButton is already exactly "Scan barcode" and already translated in all nine. Neither the automated review nor a hand-run one spotted the duplicate — it turned up only by looking for precedents first. A second phrasing for one action is a worse outcome than a late translation, and it is the one a reviewer can prevent. AGENTS.md 28,140 -> 28,705 bytes; 2,295 under the check_agents_md guard.
|
Thanks for this — retargeting the base from
The side effect is that GitHub now shows this as conflicting. That isn't a problem with your change — the branch was cut from To get it green, rebase onto current git remote add upstream https://github.com/simonoppowa/OpenNutriTracker.git
git fetch upstream
git rebase upstream/develop
git push --force-with-leaseYour actual change (the scan and create-custom actions on the no-results search state, #578) should survive that cleanly — the conflicts are almost entirely in files you didn't touch, inherited from the old base. Happy to help if the rebase turns up anything awkward. |
…imonoppowa#578) Zero-result searches previously only showed the "No results found" hint, so users had no obvious next step even though the app can scan a barcode or open a custom-food form. NoResultsWidget now takes optional onScanBarcode and onCreateCustomFood callbacks and renders "Scan barcode" and "Create custom food" buttons under a short explanatory subtitle when they're supplied. The bare "No results found" is still available for any caller that leaves both callbacks null. The four AddMealScreen call sites (All / Products / Food / Recent) pass both callbacks: barcode reuses the existing scanner-navigation helper, and create-custom reads the current imperial-units flag from the AddMealBloc before opening the same confirm-and-open-editor flow as the app-bar Add button. Semantics identifiers on both buttons for the ADB / Appium drivers described in AGENTS.md. New strings added to every ARB (English text carried into locale files until they're translated). Widget tests cover the three visible shapes: no callbacks, both callbacks, and scan-only.
- Reuse the source-specific state's usesImperialUnits instead of the factory-registered AddMealBloc (which returned a fresh, uninitialized instance and always fell back to metric). - Drop the recovery actions from the remoteSourceEmpty footer inside the products/food ListViews so it doesn't contradict the visible matches above it — the footer renders as the bare hint, actions stay on the pure empty state. - Wrap NoResultsWidget in a SingleChildScrollView so the icon+text+ buttons survive landscape and large system text scales without overflowing the tightly constrained results area.
noResultsScanBarcode copies customMealBarcodeScanButton verbatim in every locale so the two scan actions read identically. The create and subtitle strings follow the noun already used by newCustomMealLabel / customMealsMergeAction per locale. tr, uk and zh subtitles are machine translated as a starting point; the other five are as supplied in review.
…ty state With an empty query and no logged meals, the Recent tab rendered the "no foods matched that search" state even though no search ran. Use noMealsRecentlyAddedLabel there; the actioned search empty state now only appears when a non-empty query filters Recent down to nothing.
60117ba to
bfd476a
Compare
|
Thanks for the retarget and the runbook. Rebased onto Force-pushed with |
|
Thanks for the rebase and the translation pass — the ARBs are exactly right now (all nine at parity, Scan barcode copied verbatim per locale), analyze and tests are green on a local merge with develop, and the Recent-tab split is the behaviour I wanted. One small thing before I merge, on the overflow fix from the last round. The SingleChildScrollView in NoResultsWidget only scrolls when its parent bounds its height. On the All and Recent tabs it sits inside an Expanded, so it works there — but on the Products and Food tabs (add_meal_screen.dart:397 and :450) it is returned as a plain child of the tab's Column, gets unbounded height, and never scrolls; the Column overflows instead. I reproduced it in a widget test at a 260px-tall results area: 32px overflow, maxScrollExtent 0. Wrapping those two returns in Flexible(child: ...), the same way the ListView right below them already is, fixes it. Could you also change "Fixes #578" to "Part of #578"? You were upfront that partial-failure handling and the state tests are deferred, and I'd like the issue to stay open for those. Happy to take the follow-up whenever you have time. |
2 participants
Summary
Zero-result searches used to hit a wall — just a "No results found" hint, no next step. This PR turns that empty state into a useful one by surfacing the two things the app can actually do next: scan a barcode or open the custom-food form.
Type of change
Related issues
Fixes #578.
Changes
Acceptance criteria coverage from the issue
Screenshots / recordings
Not attached — I don't have a device set up locally to record right now. Layout is a `Wrap` under the existing `EmptyHint`, so buttons flow to a second row on narrow widths.
Test plan
Steps
Checklist