Skip to content

fixed issue for muliple tap were nedded to view sub issue. #9139

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
notHuman9504 wants to merge 1 commit into
base: preview
Choose a base branch
from
+9 −6
Open

fixed issue for muliple tap were nedded to view sub issue. #9139

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions apps/web/core/components/issues/issue-layouts/list/block.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,17 +143,15 @@ export const IssueBlock = observer(function IssueBlock(props: IssueBlockProps) {

const marginLeft = `${spacingLeft}px`;

const handleToggleExpand = (e: MouseEvent<HTMLButtonElement>) => {
const handleToggleExpand = async (e: MouseEvent<HTMLButtonElement>) => {
e.stopPropagation();
e.preventDefault();
if (nestingLevel >= 3) {
handleIssuePeekOverview(issue);
} else {
setExpanded((prevState) => {
if (!prevState && workspaceSlug && issue && issue.project_id)
subIssuesStore.fetchSubIssues(workspaceSlug.toString(), issue.project_id, issue.id);
return !prevState;
});
if (!isExpanded && workspaceSlug && issue.project_id)
await subIssuesStore.fetchSubIssues(workspaceSlug.toString(), issue.project_id, issue.id);
setExpanded((prevState) => !prevState);
}
};
Comment on lines +146 to 156
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 25, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard against concurrent expand/collapse operations.

The async handler has no protection against rapid successive clicks. If a user clicks the expand button multiple times while fetchSubIssues is pending, multiple API calls will be triggered and the expanded state will toggle unpredictably.

Consider adding a loading flag or disabling the button during the fetch operation.

🔒 Proposed fix with loading guard

Add a local state or use the store's loader:

+const [isTogglingExpand, setIsTogglingExpand] = useState(false);
+
 const handleToggleExpand = async (e: MouseEvent<HTMLButtonElement>) => {
   e.stopPropagation();
   e.preventDefault();
+  
+  if (isTogglingExpand) return;
+  
   if (nestingLevel >= 3) {
     handleIssuePeekOverview(issue);
   } else {
     if (!isExpanded && workspaceSlug && issue.project_id) {
+      setIsTogglingExpand(true);
       try {
         await subIssuesStore.fetchSubIssues(workspaceSlug, issue.project_id, issue.id);
         setExpanded((prevState) => !prevState);
       } catch (error) {
         // ... error handling
+      } finally {
+        setIsTogglingExpand(false);
       }
     } else {
       setExpanded((prevState) => !prevState);
     }
   }
 };

And update the button to show loading state:

 <button
   type="button"
   className="grid size-4 place-items-center rounded-xs text-placeholder hover:text-tertiary"
   onClick={handleToggleExpand}
+  disabled={isTogglingExpand}
 >
+  {isTogglingExpand ? (
+    <Spinner className="size-4" />
+  ) : (
     <ChevronRightIcon
       className={cn("size-4", {
         "rotate-90": isExpanded,
       })}
       strokeWidth={2.5}
     />
+  )}
 </button>
🤖 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 `@apps/web/core/components/issues/issue-layouts/list/block.tsx` around lines
146 - 156, The handleToggleExpand handler can fire multiple concurrent
fetchSubIssues calls and flip setExpanded unpredictably; add a loading guard
(e.g., local isLoading state or reuse subIssuesStore loader) inside
handleToggleExpand to early-return while a fetch is in progress, set the loader
true before awaiting subIssuesStore.fetchSubIssues and false in a finally block,
and update the expand/collapse button to be disabled or show a loading indicator
while isLoading so rapid clicks won't trigger multiple fetchSubIssues calls or
race setExpanded; reference handleToggleExpand, fetchSubIssues, setExpanded,
nestingLevel, and subIssuesStore when making the changes.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Add error handling for the async fetch operation.

The async fetchSubIssues call has no try-catch block. If the API call fails, the error will propagate as an unhandled promise rejection, and the expansion state will still toggle, leaving the UI in an inconsistent state (expanded chevron with no sub-issues loaded).

As per coding guidelines, async operations should use try-catch with proper error types and log errors appropriately.

🛡️ Proposed fix with error handling 
 const handleToggleExpand = async (e: MouseEvent<HTMLButtonElement>) => {
   e.stopPropagation();
   e.preventDefault();
   if (nestingLevel >= 3) {
     handleIssuePeekOverview(issue);
   } else {
-    if (!isExpanded && workspaceSlug && issue.project_id)
-      await subIssuesStore.fetchSubIssues(workspaceSlug.toString(), issue.project_id, issue.id);
-    setExpanded((prevState) => !prevState);
+    if (!isExpanded && workspaceSlug && issue.project_id) {
+      try {
+        await subIssuesStore.fetchSubIssues(workspaceSlug, issue.project_id, issue.id);
+        setExpanded((prevState) => !prevState);
+      } catch (error) {
+        setToast({
+          type: TOAST_TYPE.ERROR,
+          title: "Failed to load sub-issues",
+          message: error instanceof Error ? error.message : "An error occurred while fetching sub-issues",
+        });
+      }
+    } else {
+      setExpanded((prevState) => !prevState);
+    }
   }
 };

Note: Also removed redundant .toString() on workspaceSlug (already converted to string on line 82).

🤖 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 `@apps/web/core/components/issues/issue-layouts/list/block.tsx` around lines
146 - 156, In handleToggleExpand, wrap the await
subIssuesStore.fetchSubIssues(...) call in a try-catch and only call setExpanded
when the fetch succeeds (or fall back to a defined error state), so an API
failure doesn't leave the chevron expanded with no data; catch the error, log it
via your logger (or console.error) with context including issue.id and
workspaceSlug, and remove the redundant .toString() on workspaceSlug when
passing it to fetchSubIssues. Ensure the catch block prevents toggling expansion
on failure and surfaces or records the error appropriately.


Expand Down Expand Up @@ -321,6 +319,7 @@ export const IssueBlock = observer(function IssueBlock(props: IssueBlockProps) {
isEpic={isEpic}
/>
<div
role="presentation"
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 25, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Remove role="presentation" from interactive wrapper.

The role="presentation" attribute tells assistive technologies to ignore this element's semantic role, treating it as purely decorative. However, this element has onClick and onKeyDown handlers, making it functionally interactive (even if only to stop event propagation).

This creates a contradiction: the element is marked as non-interactive but has interactive behavior. Either:

  1. Remove role="presentation" if the div needs event handlers, or
  2. Remove the event handlers and role if the child buttons already handle all interaction

Since the div appears to be a click/keyboard event trap to prevent bubbling to the parent ControlLink, consider using a more appropriate ARIA role or no role at all.

♻️ Suggested alternatives

Option 1: Remove the role attribute (simplest):

 <div
-  role="presentation"
   className={cn("hidden", {

Option 2: If the div groups related actions, use role="group" with an accessible label:

 <div
-  role="presentation"
+  role="group"
+  aria-label="Quick actions"
   className={cn("hidden", {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
role="presentation"
<div
className={cn("hidden", {
🤖 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 `@apps/web/core/components/issues/issue-layouts/list/block.tsx` at line 322,
The wrapper div in IssueActions (the element with role="presentation" that also
has onClick and onKeyDown handlers) is marked non-interactive while handling
events; remove the contradictory role="presentation" attribute so assistive tech
sees its interactive behavior, or if you intend it purely as a grouping element,
replace role="presentation" with role="group" and add an appropriate aria-label;
ensure the event handlers on that div (onClick/onKeyDown) remain or are removed
consistently with your choice so ControlLink's semantics are not broken.

className={cn("hidden", {
"md:flex": isSidebarCollapsed,
"lg:flex": !isSidebarCollapsed,
Expand All @@ -329,6 +328,10 @@ export const IssueBlock = observer(function IssueBlock(props: IssueBlockProps) {
e.preventDefault();
e.stopPropagation();
}}
onKeyDown={(e) => {
e.preventDefault();
e.stopPropagation();
}}
Comment on lines +331 to +334
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 25, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Prevent default on all keyboard events breaks accessibility.

The onKeyDown handler calls e.preventDefault() and e.stopPropagation() for every keyboard event. This will block essential keyboard navigation including Tab (focus movement), Enter/Space (activation), Escape (closing dialogs), and arrow keys.

If the intent is to prevent specific keys from bubbling (e.g., only Enter or Space to avoid double-triggering the parent link), you should check e.key and only prevent default for those specific keys.

♿ Proposed fix for selective event handling

Only prevent activation keys that might trigger the parent ControlLink:

 onKeyDown={(e) => {
-  e.preventDefault();
-  e.stopPropagation();
+  // Only prevent Enter/Space to avoid triggering parent link
+  if (e.key === 'Enter' || e.key === ' ') {
+    e.preventDefault();
+    e.stopPropagation();
+  }
 }}

Alternatively, if you want to stop all propagation but still allow default browser behavior (like Tab navigation):

 onKeyDown={(e) => {
-  e.preventDefault();
   e.stopPropagation();
 }}
🤖 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 `@apps/web/core/components/issues/issue-layouts/list/block.tsx` around lines
331 - 334, The current onKeyDown handler in the IssueList block (the inline
onKeyDown prop defined around the ControlLink/child element) calls
e.preventDefault() and e.stopPropagation() for every key which breaks keyboard
navigation; change it so you only preventDefault()/stopPropagation() for
specific activation keys (e.g., e.key === 'Enter' or e.key === ' ' / 'Spacebar')
and return early for other keys (allowing Tab, Escape, arrow keys to behave
normally), or alternatively only call stopPropagation() without preventDefault()
when you need to stop parent handlers but preserve native focus movement.

>
{quickActions({
issue,
Expand Down