Skip to content

fix: auto-scroll focused organization card during keyboard navigation - #2041

Open
karizcloud wants to merge 1 commit into
S3DFX-CYBER:mainfrom
karizcloud:fix-keyboard-navigation-scroll
Open

karizcloud wants to merge 1 commit into
S3DFX-CYBER:mainfrom
karizcloud:fix-keyboard-navigation-scroll

Conversation

@karizcloud

@karizcloud karizcloud commented Jul 26, 2026

Copy link
Copy Markdown

Summary

This PR fixes the keyboard navigation scrolling issue in the Organizations section.

What Changed

  • Updated the card selection logic so the focused organization card is automatically scrolled into view during keyboard navigation.
  • Removed the dependency on the missing data-filtered-idx attribute by locating the focused card from the rendered card list.
  • Preserved existing keyboard navigation, search, filtering, and selection behavior.

Related Issue

Fixes #2038

Type of Change

  • Bug fix

Testing

  • Verified keyboard navigation using Arrow Up/Down/Left/Right.
  • Confirmed the focused card scrolls into view automatically.
  • Verified search and filters continue to work correctly.
  • Checked that there are no console errors.

Checklist

  • Code follows the project style.
  • Tested locally.
  • No unrelated changes included.

Review in cubic

@karizcloud
karizcloud requested a review from S3DFX-CYBER as a code owner July 26, 2026 16:27
@vercel

vercel Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

@karizcloud is attempting to deploy a commit to the s3dfx-cyber's projects Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions

Copy link
Copy Markdown
Contributor

💬 Faster Reviews & Assignments

Hi @karizcloud, for faster coordination and smoother communication, consider joining our Discord community:

👉 https://discord.gg/jYrbHKtn8t

Useful Channels

  • #issue-links-for-assignment → Share issue links for assignment help
  • #pr-links-for-review → Share PR links for mentor/maintainer review

Please avoid spamming channels or repeatedly pinging mentors/maintainers.

@github-actions

github-actions Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

⚠️ PR Validation Issues

Hi @karizcloud, your PR requires fixes before review.

Warnings

  • ⚠️ Missing contribution program declaration (GSSOC or NSOC).

Please push fixes after updating the PR.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your trial has ended. Reactivate Greptile to resume code reviews.

@github-actions

Copy link
Copy Markdown
Contributor

👋 Thanks for opening a PR, @karizcloud!

Your PR has entered the 🚦 PR Review Pipeline.

Standard PR detected — your PR will follow the standard repository review pipeline.


🔄 Review Flow

Stage Reviewer Purpose
Stage 1 🤖 Automation Validation · Duplicate Detection · AI/Slop Checks · Formatting · PR Analysis
Stage 2 👥 Repository Reviewer Code Review · Scope Validation · Quality Check
Stage 3 🔑 Project Admin / Maintainer Final Approval & Merge Decision

The automated PR analysis system will verify issue linkage, PR relevance, and contribution quality.

A pipeline status comment may appear automatically as your PR progresses.


✅ Contributor Checklist

  • Sign commits using git commit -s
  • Link a valid issue (Closes #123)
  • Keep changes focused and relevant
  • Do not include unrelated modifications
  • Ensure workflows/build/tests are passing
  • Read the appropriate contributor guide:

⚠️ Important Notes

  • Low-quality, spammy, or AI-generated PRs may be closed
  • PRs without linked issues may fail automated checks
  • Large unrelated PRs are likely to be rejected
  • Review times may vary depending on mentor/reviewer availability

Happy contributing 🚀

This message is posted automatically and only once.

@github-actions

github-actions Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

✅ DCO Sign-off Verified

Hi @karizcloud 👋

All commits in this PR contain valid Signed-off-by lines.

Thank you for following the DCO requirements 🚀

@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 861337a0-075b-43cb-af1f-2c42c922ce82

📥 Commits

Reviewing files that changed from the base of the PR and between 54edf39 and eac292b.

📒 Files selected for processing (1)
  • src/js/app.js
📜 Recent review details
⚠️ CI failures not shown inline (1)

Commit Status: Vercel: Vercel

Conclusion: failure

Authorization required to deploy.
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-06-15T18:15:28.688Z
Learnt from: arghya29
Repo: S3DFX-CYBER/GSoC-Org-Finder- PR: 1882
File: src/js/footer.js:33-33
Timestamp: 2026-06-15T18:15:28.688Z
Learning: In this repo’s JavaScript (e.g., footer.js), `globalThis` is intentionally preferred over `window` to keep code environment-agnostic and to satisfy SonarCloud static analysis. The project targets modern browsers (ES2021) with no transpilation, so `globalThis` is fully supported—do not flag `globalThis` usage as a browser compatibility concern or suggest replacing it with `window` during review.

Applied to files:

  • src/js/app.js
🪛 ast-grep (0.44.1)
src/js/app.js

[error] 648-658: React's useState should not be directly called
Context: setTimeout(() => {
const cards = document.querySelectorAll('#orgGrid article');
const card = cards[focusedIdx];
if (card) {
card.scrollIntoView({
behavior: 'smooth',
block: 'nearest',
inline: 'nearest'
});
}
}, 30)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.

(usestate-direct-usage)

🔇 Additional comments (1)
src/js/app.js (1)

650-658: LGTM!


📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Improved keyboard navigation by reliably scrolling the currently focused organization card into view.
    • Enhanced smooth, more precise scrolling behavior so the focused card stays appropriately positioned in the viewport.

Walkthrough

scrollToFocused() now selects organization cards from #orgGrid article using focusedIdx and scrolls the focused card into view with nearest smooth scrolling on both axes.

Changes

Focused card scrolling

Layer / File(s) Summary
Scroll the focused organization card
src/js/app.js
scrollToFocused() selects the focused article by its rendered position instead of a data-filtered-idx attribute, then uses smooth nearest scrolling with an inline alignment option.

Estimated code review effort: 2 (Simple) | ~5 minutes

Possibly related PRs

Suggested labels: bug, frontend

Suggested reviewers: s3dfx-cyber, itzzavdheshh

Poem

I hop through cards in a focused row,
Now hidden picks are brought to show.
Smoothly near, from side to side,
The chosen card will no longer hide.
— A scrolling rabbit 🐇

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely matches the main change: auto-scrolling the focused organization card during keyboard navigation.
Description check ✅ Passed The description covers the summary, related issue, change type, testing, and checklist, with only non-critical template items omitted.
Linked Issues check ✅ Passed The change satisfies #2038 by making the focused card scroll into view using the rendered card list in src/js/app.js.
Out of Scope Changes check ✅ Passed The PR stays within scope; the only changes are in src/js/app.js and directly support the scrolling fix.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

🚦 PR Review Pipeline

Standard PR

Stage Status
Stage 1 — Automated Checks ✅ All automated checks passed
Stage 2 — Mentor/Reviewer ⏳ Awaiting reviewer approval
Stage 3 — Maintainer 🔒 Blocked until Stage 2 passes

No active issues


Last updated: Sun, 26 Jul 2026 16:33:44 GMT

@github-actions

Copy link
Copy Markdown
Contributor

🤖 TENET Agent Review

📋 Summary

This PR addresses a bug in the Organizations section where keyboard navigation failed to automatically scroll the focused card into view. The fix updates the scrollToFocused function in src/js/app.js to locate the focused card by directly indexing the list of rendered article elements, removing a dependency on a potentially missing data-filtered-idx attribute. The approach is sound and improves UI usability.

🔐 Security Findings

No security issues found.

🧹 Code Quality

Code quality looks good.

✅ What's Done Well

  1. Effectively fixes a user experience bug related to keyboard navigation.
  2. Improves robustness by removing a dependency on a potentially unreliable or missing data-filtered-idx attribute.
  3. Enhances the scrolling behavior by adding inline: 'nearest' to scrollIntoView options, providing a more complete view of the focused element.

📝 Overall Verdict

[APPROVE] - A clear bug fix that improves UI usability and code robustness without introducing new issues.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 1 file

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Auto-approved: Focused bug fix that replaces a broken attribute-based card lookup with direct indexing, restoring scroll-to-focus during keyboard navigation. No new behavior, operational, or security tradeoffs introduced.

Re-trigger cubic

Signed-off-by: karizcloud <1006karishmakale@gmail.com>
@karizcloud
karizcloud force-pushed the fix-keyboard-navigation-scroll branch from 54edf39 to eac292b Compare July 26, 2026 16:33

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your trial has ended. Reactivate Greptile to resume code reviews.

@sonarqubecloud

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown
Contributor

🤖 TENET Agent Review

📋 Summary

This pull request addresses a bug in the Organizations section where keyboard navigation failed to automatically scroll the focused card into view. The fix updates the scrollToFocused function to locate the target card by its index within the rendered list of organization cards, rather than relying on a previously missing data-filtered-idx attribute. The approach is sound for resolving the described UI bug.

🔐 Security Findings

No security issues found.

🧹 Code Quality

  • src/js/app.js - The new card selection logic cards[focusedIdx] assumes that focusedIdx directly corresponds to the zero-based index of the desired article element within the NodeList returned by document.querySelectorAll('#orgGrid article'). This creates a strong coupling between the focusedIdx variable (managed by keyboard navigation) and the exact DOM order of all article elements. Ensure focusedIdx is consistently synchronized with the visual DOM index, especially if elements are dynamically added, removed, or reordered, or if filtering changes the visible set.

✅ What's Done Well

  • Successfully addresses a critical keyboard navigation accessibility bug.
  • Removes a dependency on a problematic and missing data-filtered-idx attribute, making the card selection more robust.
  • Improves the user experience by ensuring the focused element is always visible during navigation.

📝 Overall Verdict

[APPROVE] - The PR provides a clear fix for a UI bug with no apparent security implications.


Review powered by TENET Agent 🛡️ | Triggered automatically on PR #2041

@S3DFX-CYBER S3DFX-CYBER left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes the keyboard-navigation scroll bug from #2038, where scrollToFocused() was silently failing because it looked up the focused card via data-filtered-idx, an attribute that was never being set anywhere in app.js. The new approach indexes directly into the rendered <article> elements via document.querySelectorAll('#orgGrid article')[focusedIdx], which is simpler and actually works. Fixes #2038 is linked, DCO is signed, the title is correctly lowercase fix:, and it's a tight 1-file +9/−3 diff with no dependency or lockfile changes. No security concerns — pure DOM queries, no innerHTML or untrusted data.

On the index.html inline / src/js/app.js sync rule: I checked, and scrollToFocused — along with the entire keyboard card-navigation feature (focusedIdx, handleNavigationRight/Left/Down/Up, GRID_COLS) — exists only in src/js/app.js, not in index.html's inline script. So there's no sync violation here; the function being fixed only lives in one place. (That the feature is test-mirror-only and not in the production index.html runtime is a pre-existing architectural state, not something this PR introduces, so I'm not holding it against the change.)

The index alignment is sound: updateCardFocus() iterates the same querySelectorAll('#orgGrid article') NodeList with idx === focusedIdx, so cards[focusedIdx] in scrollToFocused() is consistent with how focus is tracked elsewhere. Adding inline: 'nearest' alongside block: 'nearest' is correct for the horizontal component of grid navigation and doesn't change vertical behavior.

Good to go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Keyboard navigation does not scroll the focused organization card into view

2 participants