Custom useTabHook for Tab Navigation

[mshriver]

The browser back/forward and direct tab navigation via hash is not working correctly in the feature branch.

patternfly/patternfly-react#11715

The PF demo for this isn't functional.

This introduces a custom hook for tab state and navigation via location hash. I've integrated on the Run and Result views.

I've also made some small updates across the Link hrefs to use the default tab location hash on initial navigation, which makes for a better experience than just updating it in effects.

Summary by Sourcery

Introduce a custom useTabHook for consistent tab navigation and hash-based routing across multiple components

New Features:

• Created a custom useTabHook to manage tab state and navigation consistently across different views
• Added support for default tab selection and hash-based routing

Enhancements:

• Refactored tab navigation logic in Run, ResultView, and JenkinsJobAnalysisView components
• Simplified tab state management by extracting common navigation logic into a reusable hook
• Improved browser back/forward navigation handling

Chores:

• Updated Link components to include default hash for consistent navigation
• Removed redundant tab navigation code from multiple components

[sourcery-ai]

Hey @mshriver - I've reviewed your changes - here's some feedback:

Overall Comments:

• Consider creating a custom hook to encapsulate the tab navigation logic, as suggested in the description.
• It might be good to validate the tabIndex value in handleHashChange against the available tabs to prevent unexpected behavior.

Here's what I looked at during the review

• 🟡 General issues: 1 issue found
• 🟢 Security: all looks good
• 🟢 Testing: all looks good
• 🟢 Complexity: all looks good
• 🟢 Documentation: all looks good

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[mshriver]

@sourcery-ai review

[sourcery-ai]

Hey @mshriver - I've reviewed your changes - here's some feedback:

Overall Comments:

• Consider adding a comment to explain the purpose and usage of the useTabHook.
• It looks like you're passing a hardcoded list of tab names to useTabHook; consider generating this list dynamically.

Here's what I looked at during the review

• 🟢 General issues: all looks good
• 🟢 Security: all looks good
• 🟢 Testing: all looks good
• 🟡 Complexity: 1 issue found
• 🟢 Documentation: all looks good

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[hugohezel]

On Run's details page, opening any of the test results in Results Tree tab bugs the tab selection.
Screencast From 2025-04-17 10-57-07.webm

[mshriver]

@hugohezel thanks! Yep, the nested tabs can't also be tracked in the URL. Thought about slicing out multiple hash references, but there's no real need or use to also include the selected result tab in the URL as well. The selected result and tree expansion can't be reached by link alone.

Classify Failures and Results Tree tabs both end up needing the skipHash property.

[mshriver]

@sourcery-ai review

[sourcery-ai]

Reviewer's Guide by Sourcery

This pull request introduces a custom useTabHook to manage tab state and navigation using the URL hash. It integrates this hook into the Run and ResultView components, updates Link components to include the default tab hash, refactors the buildTree function, and adds a defaultTab prop to the JenkinsJobAnalysisView component.

No diagrams generated as the changes look simple and do not need a visual representation.

File-Level Changes

Change	Details	Files
Introduced a custom hook, useTabHook, to manage tab state and navigation using the location hash.	Created a useTabHook hook to handle tab selection and navigation based on the URL hash. The hook takes an array of valid tab keys, a default tab, and a flag to skip hash updates. The hook returns the active tab and a function to select a tab. The hook updates the active tab based on the URL hash. The hook navigates to the default tab if the hash is invalid.	frontend/src/components/tabHook.js
Integrated the useTabHook in the Run and ResultView components to manage tab state and navigation.	Replaced the existing tab state management logic with the useTabHook in Run and ResultView components. Modified Run and ResultView to use the activeTab and onTabSelect values returned by the hook. Added a skipHash prop to ResultView to conditionally disable hash updates. Updated Run component to dynamically generate artifact tabs using useMemo and include them in the valid tab keys for useTabHook. Updated Run component to fetch results tree only when the 'results-tree' tab is active.	frontend/src/run.js frontend/src/components/result.js
Updated Link components to include the default tab hash for initial navigation.	Modified Link components to include the default tab hash in the href attribute. Updated links in resultToRow, resultToComparisonRow, FileUpload, LastPassed, and FilterHeatmapWidget to include '#summary'.	frontend/src/utilities.js frontend/src/components/fileupload.js frontend/src/components/last-passed.js frontend/src/widgets/filterheatmap.js
Refactored the buildTree function into buildResultsTree and moved it to utilities.js.	Renamed buildTree to buildResultsTree to better reflect its purpose. Moved the function to utilities.js to promote reusability.	frontend/src/run.js frontend/src/utilities.js
Added a defaultTab prop to the JenkinsJobAnalysisView component and integrated useTabHook.	Added a defaultTab prop to JenkinsJobAnalysisView. Integrated useTabHook to manage tab state. Removed the useLocation and useNavigate hooks from the component.	frontend/src/views/jenkinsjobanalysis.js

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!
• Generate a plan of action for an issue: Comment @sourcery-ai plan on
  an issue to generate a plan of action for it.

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey @mshriver - I've reviewed your changes - here's some feedback:

Overall Comments:

• Consider extracting the tree building logic into a separate module for better organization and reusability.
• The new useTabHook looks good, but ensure it's thoroughly tested, especially the edge cases and fallback behavior.

Here's what I looked at during the review

• 🟢 General issues: all looks good
• 🟢 Security: all looks good
• 🟢 Testing: all looks good
• 🟡 Complexity: 2 issues found
• 🟢 Documentation: all looks good

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

issue (complexity): Consider merging the artifact key extraction into the existing useMemo hook to compute all tab keys at once, avoiding an extra useCallback wrapper function and simplifying the code.

You can simplify the extraction of the artifact keys by avoiding an extra useCallback wrapper when you already have the artifact data. For example, merge the key extraction into a single useMemo that computes all tab keys. This reduces indirection while keeping functionality intact:

const tabKeys = useMemo(() => {
  // Creates an array of tab keys, including artifacts directly
  return ['summary', 'testHistory', 'testObject', ...artifacts.map(art => art.filename)];
}, [artifacts]);

const { activeTab, onTabSelect } = useTabHook(tabKeys, defaultTab, skipHash);

This change removes the extra artifactKeys callback and makes the key computation clear and directly dependent on artifacts.

[sourcery-ai]

issue (complexity): Consider refactoring the buildResultsTree function to extract helper functions for creating child nodes and selecting icons to reduce nesting and improve readability and maintainability of the code.

Consider refactoring the deep nested logic inside buildResultsTree by extracting helper functions. For example, you can create one helper to get or create a child node and another to select the icon based on the test result. This will simplify the main loop and reduce cognitive load.

Before:

treeResults.forEach(testResult => {
  const pathParts = processPyTestPath(cleanPath(testResult.metadata.fspath));
  let children = treeStructure;
  pathParts.forEach(dirName => {
    let child = children.find(item => item.name == dirName);
    if (!child) {
      child = {
        name: dirName,
        id: dirName,
        children: [],
        hasBadge: true,
        _stats: { count: 0, passed: 0, failed: 0, skipped: 0, error: 0, xpassed: 0, xfailed: 0 }
      };
      if (dirName.endsWith('.py')) {
        child.icon = <FileIcon />;
        child.expandedIcon = <FileIcon />;
      }
      children.push(child);
    }
    // update stats and badge...
    child._stats[testResult.result] += 1;
    child._stats.count += 1;
    const passPercent = getPassPercent(child._stats);
    const className = getBadgeClass(passPercent);
    child.customBadgeContent = `${passPercent}%`;
    child.badgeProps = { className: className };
    children = child.children;
  });
  let icon = <QuestionCircleIcon />;
  if (testResult.result === 'passed') { icon = <CheckCircleIcon />; }
  else if (testResult.result === 'failed') { icon = <TimesCircleIcon />; }
  // ... other conditions
  children.push({
    id: testResult.id,
    name: testResult.test_id,
    icon: <span className={testResult.result}>{icon}</span>,
    _testResult: testResult
  });
});

After (suggested changes):

1. Extract a helper to get or create a child node:

function getOrCreateChild(children, name) {
  let child = children.find(item => item.name === name);
  if (!child) {
    child = {
      name,
      id: name,
      children: [],
      hasBadge: true,
      _stats: { count: 0, passed: 0, failed: 0, skipped: 0, error: 0, xpassed: 0, xfailed: 0 }
    };
    if (name.endsWith('.py')) {
      child.icon = <FileIcon />;
      child.expandedIcon = <FileIcon />;
    }
    children.push(child);
  }
  return child;
}

2. Extract icon selection:

function selectIcon(result) {
  switch (result) {
    case 'passed': return <CheckCircleIcon />;
    case 'failed': return <TimesCircleIcon />;
    case 'error': return <ExclamationCircleIcon />;
    case 'skipped': return <ChevronRightIcon />;
    case 'xfailed': return <CheckCircleIcon />;
    case 'xpassed': return <TimesCircleIcon />;
    default: return <QuestionCircleIcon />;
  }
}

3. Refactor buildResultsTree:

export const buildResultsTree = (treeResults) => {
  const getPassPercent = (stats) => {
    return stats.count > 0
      ? Math.round(((stats.passed + stats.xfailed) / stats.count) * 100)
      : 'N/A';
  };

  const getBadgeClass = (passPercent) => {
    if (passPercent > 90) return 'passed';
    if (passPercent > 75) return 'error';
    return 'failed';
  };

  let treeStructure = [];
  treeResults.forEach(testResult => {
    const pathParts = processPyTestPath(cleanPath(testResult.metadata.fspath));
    let children = treeStructure;

    pathParts.forEach(dirName => {
      const child = getOrCreateChild(children, dirName);
      // Update stats and badge
      child._stats[testResult.result] += 1;
      child._stats.count += 1;
      const passPercent = getPassPercent(child._stats);
      child.customBadgeContent = `${passPercent}%`;
      child.badgeProps = { className: getBadgeClass(passPercent) };

      children = child.children;
    });

    // Add leaf node with appropriate icon
    children.push({
      id: testResult.id,
      name: testResult.test_id,
      icon: <span className={testResult.result}>{selectIcon(testResult.result)}</span>,
      _testResult: testResult
    });
  });
  return treeStructure;
};

These steps keep functionality intact while reducing nesting and making each piece easier to test and maintain.