-
Notifications
You must be signed in to change notification settings - Fork 337
Add open_in_new_tab parameter to me.navigate #1325
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
Changes from all commits
5365f9f
f94844d
7d81ab8
c12e0e2
3341157
58a646b
85d8365
f5c78bf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| import mesop as me | ||
|
|
||
|
|
||
| @me.page(path="/testing/navigate_new_tab") | ||
| def page(): | ||
| me.text("Navigate to New Tab Example", type="headline-5") | ||
| me.text("Click the buttons below to test navigation in new tabs:") | ||
|
|
||
| with me.box(style=me.Style(margin=me.Margin.all(15))): | ||
| me.button("Open about page in new tab", on_click=navigate_to_about_new_tab) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When I try to test this, a new tab opens to the right page. However the current page also changes to the same page as the new tab.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 85d8365 - the issue was that
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tested it out and it still shows the same behavior. |
||
|
|
||
| with me.box(style=me.Style(margin=me.Margin.all(15))): | ||
| me.button("Open about page in same tab", on_click=navigate_to_about_same_tab) | ||
|
|
||
| with me.box(style=me.Style(margin=me.Margin.all(15))): | ||
| me.button( | ||
| "Open external URL in new tab", on_click=navigate_to_external_new_tab | ||
| ) | ||
|
|
||
| with me.box(style=me.Style(margin=me.Margin.all(15))): | ||
| me.button( | ||
| "Open with query params in new tab", | ||
| on_click=navigate_with_query_params_new_tab, | ||
| ) | ||
|
|
||
|
|
||
| def navigate_to_about_new_tab(e: me.ClickEvent): | ||
| me.navigate("/testing/navigate_new_tab_target", open_in_new_tab=True) | ||
|
|
||
|
|
||
| def navigate_to_about_same_tab(e: me.ClickEvent): | ||
| me.navigate("/testing/navigate_new_tab_target", open_in_new_tab=False) | ||
|
|
||
|
|
||
| def navigate_to_external_new_tab(e: me.ClickEvent): | ||
| me.navigate("https://example.com", open_in_new_tab=True) | ||
|
|
||
|
|
||
| def navigate_with_query_params_new_tab(e: me.ClickEvent): | ||
| me.navigate( | ||
| "/testing/navigate_new_tab_target", | ||
| query_params={"search": "test", "page": "1"}, | ||
| open_in_new_tab=True, | ||
| ) | ||
|
|
||
|
|
||
| @me.page(path="/testing/navigate_new_tab_target") | ||
| def navigate_new_tab_target(): | ||
| me.text("Navigate New Tab Target Page", type="headline-4") | ||
| me.text("This is the target page opened from the navigation example.") | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,112 @@ | ||
| import {test, expect} from '@playwright/test'; | ||
|
|
||
| test('navigate with open_in_new_tab - relative URL', async ({page, context}) => { | ||
| await page.goto('/testing/navigate_new_tab'); | ||
|
|
||
| // Get the current pages before clicking | ||
| const pagesBefore = context.pages().length; | ||
|
|
||
| // Click button to open about page in new tab | ||
| await page.getByRole('button', {name: 'Open about page in new tab'}).click(); | ||
|
|
||
| // Wait for new tab to open | ||
| await context.waitForEvent('page'); | ||
|
|
||
| // Check that a new page was opened | ||
| const pagesAfter = context.pages().length; | ||
| expect(pagesAfter).toBe(pagesBefore + 1); | ||
|
|
||
| // Get the new page | ||
| const newPage = context.pages()[pagesAfter - 1]; | ||
|
|
||
| // Wait for navigation in new tab | ||
| await newPage.waitForURL('**/testing/navigate_new_tab_target'); | ||
|
|
||
| // Verify the content in the new page | ||
| expect(await newPage.getByText('Navigate New Tab Target Page').textContent()).toContain( | ||
| 'Navigate New Tab Target Page', | ||
| ); | ||
|
|
||
| // Verify original page is still on the same URL | ||
| expect(page.url()).toContain('/testing/navigate_new_tab'); | ||
| }); | ||
|
|
||
| test('navigate with open_in_new_tab - external URL', async ({ | ||
| page, | ||
| context, | ||
| }) => { | ||
| await page.goto('/testing/navigate_new_tab'); | ||
|
|
||
| // Get the current pages before clicking | ||
| const pagesBefore = context.pages().length; | ||
|
|
||
| // Click button to open external URL in new tab | ||
| await page | ||
| .getByRole('button', {name: 'Open external URL in new tab'}) | ||
| .click(); | ||
|
|
||
| // Wait for new tab to open | ||
| await context.waitForEvent('page'); | ||
|
|
||
| // Check that a new page was opened | ||
| const pagesAfter = context.pages().length; | ||
| expect(pagesAfter).toBe(pagesBefore + 1); | ||
|
|
||
| // Get the new page | ||
| const newPage = context.pages()[pagesAfter - 1]; | ||
|
|
||
| // Wait for navigation in new tab to example.com | ||
| await newPage.waitForURL('https://example.com/**', {timeout: 5000}); | ||
|
|
||
| // Verify original page is still on the same URL | ||
| expect(page.url()).toContain('/testing/navigate_new_tab'); | ||
| }); | ||
|
|
||
| test('navigate with open_in_new_tab false - same tab', async ({page}) => { | ||
| await page.goto('/testing/navigate_new_tab'); | ||
|
|
||
| // Click button to open about page in same tab | ||
| await page | ||
| .getByRole('button', {name: 'Open about page in same tab'}) | ||
| .click(); | ||
|
|
||
| // Wait for navigation in same tab | ||
| await page.waitForURL('**/testing/navigate_new_tab_target'); | ||
|
|
||
| // Verify the content changed in the same tab | ||
| expect(await page.getByText('Navigate New Tab Target Page').textContent()).toContain( | ||
| 'Navigate New Tab Target Page', | ||
| ); | ||
| }); | ||
|
|
||
| test('navigate with query params in new tab', async ({page, context}) => { | ||
| await page.goto('/testing/navigate_new_tab'); | ||
|
|
||
| // Get the current pages before clicking | ||
| const pagesBefore = context.pages().length; | ||
|
|
||
| // Click button to open with query params in new tab | ||
| await page | ||
| .getByRole('button', {name: 'Open with query params in new tab'}) | ||
| .click(); | ||
|
|
||
| // Wait for new tab to open | ||
| await context.waitForEvent('page'); | ||
|
|
||
| // Check that a new page was opened | ||
| const pagesAfter = context.pages().length; | ||
| expect(pagesAfter).toBe(pagesBefore + 1); | ||
|
|
||
| // Get the new page | ||
| const newPage = context.pages()[pagesAfter - 1]; | ||
|
|
||
| // Wait for navigation in new tab | ||
| await newPage.waitForURL('**/testing/navigate_new_tab_target?search=test&page=1'); | ||
|
|
||
| // Verify query params are in the URL | ||
| expect(newPage.url()).toContain('search=test'); | ||
| expect(newPage.url()).toContain('page=1'); | ||
|
|
||
| // Verify original page is still on the same URL | ||
| expect(page.url()).toContain('/testing/navigate_new_tab'); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -119,11 +119,20 @@ export class Shell { | |
| }, | ||
| onCommand: async (command) => { | ||
| if (command.hasNavigate()) { | ||
| const url = command.getNavigate()!.getUrl()!; | ||
| if (url.startsWith('http://') || url.startsWith('https://')) { | ||
| const navigateCommand = command.getNavigate()!; | ||
| const url = navigateCommand.getUrl()!; | ||
| const openInNewTab = navigateCommand.getOpenInNewTab(); | ||
|
|
||
| if (openInNewTab) { | ||
| // For relative URLs, resolve to absolute URL before opening in new tab | ||
| const absoluteUrl = url.startsWith('http://') || url.startsWith('https://') | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's potentially other url formats that are absolute. Is there a more robust way to distinguish this? This is ok to ignore since we use similar pattern for the other case. |
||
| ? url | ||
| : new URL(url, window.location.href).href; | ||
| window.open(absoluteUrl, '_blank'); | ||
| } else if (url.startsWith('http://') || url.startsWith('https://')) { | ||
| window.location.href = url; | ||
| } else { | ||
| await this.router.navigateByUrl(command.getNavigate()!.getUrl()!); | ||
| await this.router.navigateByUrl(url); | ||
| this.channel.resetOverridedTitle(); | ||
| } | ||
| } else if (command.hasScrollIntoView()) { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.