Skip to content
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.

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

[JENKINS-64581] Middle-click or Ctrl+click a build in a trend chart to open the build in a new tab. #381

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

akash-manna-sky
Copy link

@akash-manna-sky akash-manna-sky commented Feb 20, 2025

Added standard browser features like Ctrl+Click (open in the new tab), Shift+Click (open in the new window), and right-click context menus to navigate to a build's warning page.

Testing done

Submitter checklist

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

@akash-manna-sky
Copy link
Author

Hello @uhafner here is the draft PR

@akash-manna-sky akash-manna-sky marked this pull request as ready for review February 20, 2025 12:06
if (event.ctrlKey || event.which === 2) { // Ctrl+Click or Middle-click
window.open(url, '_blank'); // Open in new tab/window
} else if (event.shiftKey) { // Shift+Click
window.open(url, '_blank', 'noopener,noreferrer'); // Open in a new window (more isolated)

Choose a reason for hiding this comment

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

What's the purpose of "noopener,noreferrer" here? Those would usually be for security isolation between web sites, but the link is to the same Jenkins controller anyway.

@akash-manna-sky
Copy link
Author

@KalleOlaviNiemitalo I removed "noopener, noreferrer" parameters since they are not needed for internal Jenkins Navigation. Please verify the changes.

@akash-manna-sky
Copy link
Author

Hello @uhafner can you review the changes ?

@uhafner
Copy link
Member

uhafner commented Feb 22, 2025

Did you manage to test it in a chart within Jenkins yet?

@akash-manna-sky
Copy link
Author

No... I can't manage to test it in a chart within Jenkins using Docker. Still it is showing same error.

@uhafner uhafner added the enhancement Enhancement of existing functionality label Feb 22, 2025
@uhafner
Copy link
Member

uhafner commented Feb 22, 2025

No... I can't manage to test it in a chart within Jenkins using Docker. Still it is showing same error.

Then it does not make sense to check the "test" task in the description 👎

@uhafner
Copy link
Member

uhafner commented Feb 22, 2025

I will deploy the PR into my instance and try to verify the fix...

@akash-manna-sky
Copy link
Author

Hello @uhafner were you able to deploy the PR on your instance?

@uhafner uhafner marked this pull request as draft February 23, 2025 18:13
@uhafner
Copy link
Member

uhafner commented Feb 23, 2025

Yes, but it does not work. It fails at:

    let chartPlaceHolder = document.getElementById(chartDivId);
    if (!chartPlaceHolder || !chartPlaceHolder.model) {
        console.warn('Chart placeholder or model not found for:', chartDivId);
        return;
    }

It also pop ups a menu rather than to open a page. But I have a MacBook without a mouse. I think it is required that you are testing this fix on your machine first!

@uhafner
Copy link
Member

uhafner commented Feb 23, 2025

Why did you not follow the additional steps in the chat?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants