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

Alerting: Fix reusing last url in tab when reopening a new tab in rule detail a… #79801

Merged
merged 1 commit into from
Jan 2, 2024

Conversation

soniaAguilarPeiron
Copy link
Member

@soniaAguilarPeiron soniaAguilarPeiron commented Dec 21, 2023

What is this feature?

This PR fixes some buttons reusing the last url in the tab recently opened instead of opening the url assigned to these hrefs
In particular this happened when accessing the alerting list page on an instance and clicking:

  • See Graph
  • Go to dashboard
  • Go to panel
  • Silence

The problem was we were using __blank instead of _blank , so the new tab was reusing this existing __blank named tab instead of opening a new one (as _blank option does).

Who is this feature for?

All users.

Special notes for your reviewer:

Before the fix:

before-fix-tab-reoopening-last-one.mp4

After the fix:

after-fix-tab-reoopening-last-one.mp4

Please check that:

  • It works as expected from a user's perspective.
  • If this is a pre-GA feature, it is behind a feature toggle.
  • The docs are updated, and if this is a notable improvement, it's added to our What's New doc.

@soniaAguilarPeiron soniaAguilarPeiron added this to the 10.3.x milestone Dec 21, 2023
@soniaAguilarPeiron soniaAguilarPeiron self-assigned this Dec 21, 2023
@soniaAguilarPeiron soniaAguilarPeiron requested a review from a team as a code owner December 21, 2023 15:28
@soniaAguilarPeiron soniaAguilarPeiron requested review from gillesdemey and konrad147 and removed request for a team December 21, 2023 15:28
@soniaAguilarPeiron soniaAguilarPeiron force-pushed the alerting/fix-opening-last-tab branch 3 times, most recently from 7ffa8b5 to 7c0be0d Compare December 21, 2023 16:09
@soniaAguilarPeiron
Copy link
Member Author

/deploy-to-hg

@grafana grafana deleted a comment from ephemeral-instances-bot bot Dec 21, 2023
@grafana grafana deleted a comment from ephemeral-instances-bot bot Dec 21, 2023
@ephemeral-instances-bot
Copy link

  • Preparing your instance. A comment containing your instance's url will be added to this PR when the instance is ready.
  • Your instance will be ready in ~10 minutes.
  • Check the GitHub actions tab to follow the workflow progress
  • Slack channel: #proj-ephemeral-hg-instances
  • Building instance with alerting/fix-opening-last-tab oss branch and main enterprise branch. How to choose a branch

@ephemeral-instances-bot
Copy link

@gillesdemey
Copy link
Member

I'm pretty sure that's not supposed to be the default behaviour of a browser; there might be something else at play here? Can we dig a bit deeper to find out why it's just re-opening that last tab instead of opening a new tab with the target URL?

@soniaAguilarPeiron
Copy link
Member Author

soniaAguilarPeiron commented Jan 2, 2024

I'm pretty sure that's not supposed to be the default behaviour of a browser; there might be something else at play here? Can we dig a bit deeper to find out why it's just re-opening that last tab instead of opening a new tab with the target URL?

you're right! even these changes fixed the problem, I found the real cause of opening the same tab anytime...
In the code we have __blank instead of _blank !!

Copy link
Member

@gillesdemey gillesdemey left a comment

Choose a reason for hiding this comment

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

Good catch!

@soniaAguilarPeiron soniaAguilarPeiron merged commit 1654df2 into main Jan 2, 2024
15 checks passed
@soniaAguilarPeiron soniaAguilarPeiron deleted the alerting/fix-opening-last-tab branch January 2, 2024 09:51
@summerwollin summerwollin modified the milestones: 10.3.x, 10.3.0 Jan 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants