Fake app phase2 - #1693
Open
avzz-19 wants to merge 1 commit into
Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe PR restructures fake-app details into grouped findings with a signal drawer, updates fake-app header and sticky layouts, adjusts inventory header presentation, and expands related model, fixture, translation, and test coverage. ChangesStoreknox fake-app findings groups and signal drawer
Fake-app header and sticky layout adjustments
Inventory header and legacy card layout
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying irenestaging with
|
| Latest commit: |
572721e
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://902b3d68.irenestaging.pages.dev |
| Branch Preview URL: | https://fake-app-phase2.irenestaging.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
app/components/storeknox/fake-apps/findings-group/index.ts (1)
2-11: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse one shared
ScoreLeveltype forbadgeLevel.Line 10 duplicates the union already defined in
findings-signal-row, which can drift over time.Suggested diff
-import type { SignalData } from '../findings-signal-row'; +import type { ScoreLevel, SignalData } from '../findings-signal-row'; -export type { ScoreLevel } from '../findings-signal-row'; +export type { ScoreLevel } from '../findings-signal-row'; @@ - badgeLevel: 'high' | 'medium' | 'low'; + badgeLevel: ScoreLevel;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/components/storeknox/fake-apps/findings-group/index.ts` around lines 2 - 11, The FindingsGroupData definition duplicates the badgeLevel union instead of reusing the shared ScoreLevel type from findings-signal-row. Update the FindingsGroupData.badgeLevel field to use ScoreLevel, keeping the existing re-export in place, so the type stays consistent with the rest of the findings models.tests/integration/components/storeknox/fake-apps/findings-group-test.js (1)
106-113: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the click target selection deterministic.
Lines 106-113 can pass
undefinedtoclick, causing noisy failures; use a direct selector and assert it exists first.Suggested diff
- const firstExpandBtn = document - .querySelector('[data-test-storeknoxFakeAppsFindingsGroup-root]') - ?.querySelectorAll( - '[data-test-storeknoxFakeAppsFindingsSignalRow-expandBtn]' - )[0]; - - await click(firstExpandBtn); + assert + .dom('[data-test-storeknoxFakeAppsFindingsSignalRow-expandBtn]') + .exists({ count: 2 }); + await click('[data-test-storeknoxFakeAppsFindingsSignalRow-expandBtn]');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/integration/components/storeknox/fake-apps/findings-group-test.js` around lines 106 - 113, The click target lookup in the findings group test is nondeterministic because it indexes into a queried NodeList and may pass undefined to click; update the selection in the test around firstExpandBtn to use a direct selector for the expand button and assert the element exists before clicking it. Keep the change localized to the test helper logic in findings-group-test.js so the click always targets a real element and failures are explicit.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/components/storeknox/fake-apps/details/index.ts`:
- Around line 174-226: The brandIdentityGroup getter is returning an empty group
object when none of the brand-related signals survive filtering, unlike the
other group getters. Update brandIdentityGroup in the details class to return
null whenever the filtered signals array is empty, and only build the
FindingsGroupData object when there is at least one SignalData entry after
makeMatchSignal/filter/sort.
- Around line 27-36: The current scoreToLevel heuristic mixes percentage scores
and normalized scores, which makes boundary values like 1 ambiguous. Split the
handling in app/components/storeknox/fake-apps/details by using the normalized
path only for per-rule aiScores.*Rule values, and a percentage-based path for
semanticAnalysisScore, binarySimilarityScore, and binaryRiskScore. Update the
relevant scoring helper(s) so the callers use the correct normalization instead
of one shared check.
In `@app/components/storeknox/fake-apps/findings-signal-row/index.hbs`:
- Around line 1-8: The clickable row in findings-signal-row is mouse-only;
`role='button'` on the `AkStack` does not make it keyboard-focusable or
operable. Update the `AkStack` in this template to support keyboard access by
making it focusable and handling keyboard activation alongside `@onClick`, so
users can trigger it with Enter/Space as well as mouse clicks.
In `@app/components/storeknox/fake-apps/signal-detail-drawer/index.scss`:
- Around line 4-7: The drawer content in the signal detail drawer is using a
fixed width, which can overflow on smaller screens. Update the styling in the
signal-detail-drawer SCSS so the content is responsive instead of hard-coding
620px, using the existing drawer/content selector in index.scss and preserving
the current border, margin, and padding behavior.
In `@app/styles/_component-variables.scss`:
- Around line 2528-2529: The `--storeknox-fake-apps-findings-card-border` token
is currently defined as only a color, so it won’t work when used as a full
`border` value. Update the variable definition in `_component-variables.scss` to
a complete border shorthand value, and keep
`--storeknox-fake-apps-findings-card-ignored-text` unchanged. Use the existing
`--storeknox-fake-apps-findings-card-border` symbol so all consumers render the
border correctly.
---
Nitpick comments:
In `@app/components/storeknox/fake-apps/findings-group/index.ts`:
- Around line 2-11: The FindingsGroupData definition duplicates the badgeLevel
union instead of reusing the shared ScoreLevel type from findings-signal-row.
Update the FindingsGroupData.badgeLevel field to use ScoreLevel, keeping the
existing re-export in place, so the type stays consistent with the rest of the
findings models.
In `@tests/integration/components/storeknox/fake-apps/findings-group-test.js`:
- Around line 106-113: The click target lookup in the findings group test is
nondeterministic because it indexes into a queried NodeList and may pass
undefined to click; update the selection in the test around firstExpandBtn to
use a direct selector for the expand button and assert the element exists before
clicking it. Keep the change localized to the test helper logic in
findings-group-test.js so the click always targets a real element and failures
are explicit.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 694539d8-419b-4b6a-acfa-f8f0bc1d45ae
📒 Files selected for processing (23)
app/components/storeknox/fake-apps/details/index.hbsapp/components/storeknox/fake-apps/details/index.tsapp/components/storeknox/fake-apps/findings-card/index.hbsapp/components/storeknox/fake-apps/findings-card/index.scssapp/components/storeknox/fake-apps/findings-card/index.tsapp/components/storeknox/fake-apps/findings-group/index.hbsapp/components/storeknox/fake-apps/findings-group/index.scssapp/components/storeknox/fake-apps/findings-group/index.tsapp/components/storeknox/fake-apps/findings-signal-row/index.hbsapp/components/storeknox/fake-apps/findings-signal-row/index.scssapp/components/storeknox/fake-apps/findings-signal-row/index.tsapp/components/storeknox/fake-apps/signal-detail-drawer/index.hbsapp/components/storeknox/fake-apps/signal-detail-drawer/index.scssapp/components/storeknox/fake-apps/signal-detail-drawer/index.tsapp/models/sk-fake-app.tsapp/styles/_component-variables.scssmirage/factories/sk-fake-app.tstests/integration/components/storeknox/fake-apps/details-test.jstests/integration/components/storeknox/fake-apps/findings-group-test.jstests/integration/components/storeknox/fake-apps/findings-signal-row-test.jstests/integration/components/storeknox/fake-apps/signal-detail-drawer-test.jstranslations/en.jsontranslations/ja.json
💤 Files with no reviewable changes (1)
- app/components/storeknox/fake-apps/findings-card/index.ts
avzz-19
force-pushed
the
fake-app-phase2
branch
3 times, most recently
from
June 25, 2026 10:32
5ab8553 to
3bdb491
Compare
avzz-19
force-pushed
the
fake-app-phase2
branch
2 times, most recently
from
June 25, 2026 10:39
3587405 to
d59d239
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/components/storeknox/fake-apps/findings-group/index.ts (1)
2-3: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMerge duplicate type imports from the same module.
Use a single type import for
SignalDataandScoreLevelto satisfy lint/static-analysis and reduce noise.Suggested diff
-import type { SignalData } from '../findings-signal-row'; -import type { ScoreLevel } from '../findings-signal-row'; +import type { SignalData, ScoreLevel } from '../findings-signal-row';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/components/storeknox/fake-apps/findings-group/index.ts` around lines 2 - 3, Merge the duplicate type imports in the findings-group module by combining SignalData and ScoreLevel into a single type-only import from findings-signal-row. Update the import section near the top of the file to use one import statement for both types, keeping the module path unchanged, so the lint/static-analysis rule is satisfied and the import block is cleaner.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@app/components/storeknox/fake-apps/findings-group/index.ts`:
- Around line 2-3: Merge the duplicate type imports in the findings-group module
by combining SignalData and ScoreLevel into a single type-only import from
findings-signal-row. Update the import section near the top of the file to use
one import statement for both types, keeping the module path unchanged, so the
lint/static-analysis rule is satisfied and the import block is cleaner.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: df8a9087-7e71-4ad1-a588-7d906fcebba9
📒 Files selected for processing (24)
app/components/storeknox/fake-apps/details/index.hbsapp/components/storeknox/fake-apps/details/index.scssapp/components/storeknox/fake-apps/details/index.tsapp/components/storeknox/fake-apps/findings-card/index.hbsapp/components/storeknox/fake-apps/findings-card/index.scssapp/components/storeknox/fake-apps/findings-card/index.tsapp/components/storeknox/fake-apps/findings-group/index.hbsapp/components/storeknox/fake-apps/findings-group/index.scssapp/components/storeknox/fake-apps/findings-group/index.tsapp/components/storeknox/fake-apps/findings-signal-row/index.hbsapp/components/storeknox/fake-apps/findings-signal-row/index.scssapp/components/storeknox/fake-apps/findings-signal-row/index.tsapp/components/storeknox/fake-apps/signal-detail-drawer/index.hbsapp/components/storeknox/fake-apps/signal-detail-drawer/index.scssapp/components/storeknox/fake-apps/signal-detail-drawer/index.tsapp/models/sk-fake-app.tsapp/styles/_component-variables.scssmirage/factories/sk-fake-app.tstests/integration/components/storeknox/fake-apps/details-test.jstests/integration/components/storeknox/fake-apps/findings-group-test.jstests/integration/components/storeknox/fake-apps/findings-signal-row-test.jstests/integration/components/storeknox/fake-apps/signal-detail-drawer-test.jstranslations/en.jsontranslations/ja.json
💤 Files with no reviewable changes (2)
- app/components/storeknox/fake-apps/details/index.scss
- app/components/storeknox/fake-apps/findings-card/index.ts
✅ Files skipped from review due to trivial changes (5)
- tests/integration/components/storeknox/fake-apps/signal-detail-drawer-test.js
- app/components/storeknox/fake-apps/findings-signal-row/index.hbs
- app/components/storeknox/fake-apps/findings-group/index.scss
- app/components/storeknox/fake-apps/findings-card/index.scss
- app/components/storeknox/fake-apps/findings-signal-row/index.scss
🚧 Files skipped from review as they are similar to previous changes (15)
- app/components/storeknox/fake-apps/signal-detail-drawer/index.scss
- tests/integration/components/storeknox/fake-apps/findings-signal-row-test.js
- tests/integration/components/storeknox/fake-apps/findings-group-test.js
- app/components/storeknox/fake-apps/signal-detail-drawer/index.ts
- app/components/storeknox/fake-apps/signal-detail-drawer/index.hbs
- app/components/storeknox/fake-apps/findings-signal-row/index.ts
- app/styles/_component-variables.scss
- app/components/storeknox/fake-apps/findings-group/index.hbs
- translations/en.json
- app/components/storeknox/fake-apps/details/index.hbs
- translations/ja.json
- tests/integration/components/storeknox/fake-apps/details-test.js
- app/models/sk-fake-app.ts
- mirage/factories/sk-fake-app.ts
- app/components/storeknox/fake-apps/details/index.ts
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/components/storeknox/fake-apps/details-header/index.scss`:
- Around line 73-76: The .package-name styling in the details-header SCSS is
missing the nowrap requirement, so the ellipsis truncation will not actually
apply. Update the .package-name rule to include white-space: nowrap alongside
text-overflow: ellipsis, overflow: hidden, and max-width so long package names
stay on one line and truncate correctly.
In `@app/components/storeknox/fake-apps/list-item-card/index.scss`:
- Around line 15-18: The .app-info-logo-placeholder styles no longer keep the
fallback avatar at a fixed 58px square, so it can stretch in flex layouts and
distort the circle. Update the styles in the .app-info-logo-placeholder rule to
use fixed width and height matching the real logo, and prevent flex
growth/shrink so the placeholder stays circular and consistent with the avatar
it replaces.
In `@app/components/storeknox/inventory-details/header/index.scss`:
- Around line 31-34: The fallback logo placeholder is larger than the real logo,
causing a layout shift when the image fails to load. Update the
`.app-info-logo-placeholder` styles in the `index.scss` header component so its
box matches `.app-info-logo` exactly, and keep the placeholder sizing consistent
with the image element used in `index.hbs` after the error handler swaps it in.
In
`@app/components/storeknox/inventory-details/unscanned-version/header/index.scss`:
- Around line 4-12: The sticky offset in the header tabs styles is hard-coded
and should be replaced with a shared layout token or CSS variable so it tracks
the actual height of the parent header. Update the `header/index.scss` rules for
the sticky tabs (including the `.not-appknox` variant) to consume the same
dynamic offset used by
`app/components/storeknox/inventory-details/header/index.hbs`, rather than fixed
pixel values. Prefer a single reusable variable defined at the layout level so
changes from banners, translations, or wrapping stay in sync.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 9604d962-4dd3-4e6b-ac8d-4fb1d7357f60
📒 Files selected for processing (37)
app/components/knox-iq/project-card/index.hbsapp/components/storeknox/fake-apps/details-header/index.hbsapp/components/storeknox/fake-apps/details-header/index.scssapp/components/storeknox/fake-apps/details/index.hbsapp/components/storeknox/fake-apps/details/index.scssapp/components/storeknox/fake-apps/details/index.tsapp/components/storeknox/fake-apps/fake-app-list/header/index.hbsapp/components/storeknox/fake-apps/fake-app-list/index.scssapp/components/storeknox/fake-apps/findings-card/index.hbsapp/components/storeknox/fake-apps/findings-card/index.scssapp/components/storeknox/fake-apps/findings-card/index.tsapp/components/storeknox/fake-apps/findings-group/index.hbsapp/components/storeknox/fake-apps/findings-group/index.scssapp/components/storeknox/fake-apps/findings-group/index.tsapp/components/storeknox/fake-apps/findings-signal-row/index.hbsapp/components/storeknox/fake-apps/findings-signal-row/index.scssapp/components/storeknox/fake-apps/findings-signal-row/index.tsapp/components/storeknox/fake-apps/list-item-card/index.scssapp/components/storeknox/fake-apps/original-app-info/index.hbsapp/components/storeknox/fake-apps/original-app-info/index.scssapp/components/storeknox/fake-apps/original-app-info/skeleton/index.hbsapp/components/storeknox/fake-apps/signal-detail-drawer/index.hbsapp/components/storeknox/fake-apps/signal-detail-drawer/index.scssapp/components/storeknox/fake-apps/signal-detail-drawer/index.tsapp/components/storeknox/inventory-details/header/actions/index.hbsapp/components/storeknox/inventory-details/header/index.hbsapp/components/storeknox/inventory-details/header/index.scssapp/components/storeknox/inventory-details/unscanned-version/header/index.scssapp/models/sk-fake-app.tsapp/styles/_component-variables.scssmirage/factories/sk-fake-app.tstests/integration/components/storeknox/fake-apps/details-test.jstests/integration/components/storeknox/fake-apps/findings-group-test.jstests/integration/components/storeknox/fake-apps/findings-signal-row-test.jstests/integration/components/storeknox/fake-apps/signal-detail-drawer-test.jstranslations/en.jsontranslations/ja.json
💤 Files with no reviewable changes (1)
- app/components/storeknox/fake-apps/details/index.scss
✅ Files skipped from review due to trivial changes (8)
- app/components/storeknox/fake-apps/signal-detail-drawer/index.scss
- app/components/storeknox/inventory-details/header/actions/index.hbs
- app/components/storeknox/fake-apps/fake-app-list/index.scss
- app/components/storeknox/fake-apps/original-app-info/index.scss
- tests/integration/components/storeknox/fake-apps/findings-signal-row-test.js
- app/components/storeknox/fake-apps/findings-card/index.scss
- app/components/storeknox/fake-apps/original-app-info/index.hbs
- app/components/storeknox/fake-apps/findings-signal-row/index.ts
🚧 Files skipped from review as they are similar to previous changes (17)
- app/components/storeknox/fake-apps/findings-group/index.hbs
- app/components/storeknox/fake-apps/findings-signal-row/index.scss
- app/components/storeknox/fake-apps/signal-detail-drawer/index.ts
- tests/integration/components/storeknox/fake-apps/findings-group-test.js
- app/components/storeknox/fake-apps/findings-signal-row/index.hbs
- app/components/storeknox/fake-apps/signal-detail-drawer/index.hbs
- app/components/storeknox/fake-apps/findings-group/index.ts
- app/components/storeknox/fake-apps/findings-group/index.scss
- app/components/storeknox/fake-apps/details/index.hbs
- tests/integration/components/storeknox/fake-apps/signal-detail-drawer-test.js
- translations/ja.json
- translations/en.json
- app/models/sk-fake-app.ts
- app/components/storeknox/fake-apps/findings-card/index.hbs
- mirage/factories/sk-fake-app.ts
- app/components/storeknox/fake-apps/details/index.ts
- tests/integration/components/storeknox/fake-apps/details-test.js
avzz-19
force-pushed
the
fake-app-phase2
branch
2 times, most recently
from
June 30, 2026 06:38
66c690c to
260b203
Compare
Yibaebi
requested changes
Jul 10, 2026
avzz-19
force-pushed
the
fake-app-phase2
branch
from
August 5, 2026 09:16
83bb695 to
a4facaa
Compare
avzz-19
force-pushed
the
fake-app-phase2
branch
from
August 5, 2026 09:22
a4facaa to
572721e
Compare
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.