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

Add absolute option to ApplicationStart.getUrlForApp #57193

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,16 @@

## ApplicationStart.getUrlForApp() method

Returns a relative URL to a given app, including the global base path.
Returns an URL to a given app, including the global base path. By default, the URL is relative (/basePath/app/my-app). Use the `absolute` option to generate an absolute url (http://host:port/basePath/app/my-app)

Note that when generating absolute urls, the protocol, host and port are determined from the browser location.

<b>Signature:</b>

```typescript
getUrlForApp(appId: string, options?: {
path?: string;
absolute?: boolean;
}): string;
```

Expand All @@ -19,7 +22,7 @@ getUrlForApp(appId: string, options?: {
| Parameter | Type | Description |
| --- | --- | --- |
| appId | <code>string</code> | |
| options | <code>{</code><br/><code> path?: string;</code><br/><code> }</code> | |
| options | <code>{</code><br/><code> path?: string;</code><br/><code> absolute?: boolean;</code><br/><code> }</code> | |

<b>Returns:</b>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export interface ApplicationStart

| Method | Description |
| --- | --- |
| [getUrlForApp(appId, options)](./kibana-plugin-public.applicationstart.geturlforapp.md) | Returns a relative URL to a given app, including the global base path. |
| [getUrlForApp(appId, options)](./kibana-plugin-public.applicationstart.geturlforapp.md) | Returns an URL to a given app, including the global base path. By default, the URL is relative (/basePath/app/my-app). Use the <code>absolute</code> option to generate an absolute url (http://host:port/basePath/app/my-app)<!-- -->Note that when generating absolute urls, the protocol, host and port are determined from the browser location. |
| [navigateToApp(appId, options)](./kibana-plugin-public.applicationstart.navigatetoapp.md) | Navigate to a given app |
| [registerMountContext(contextName, provider)](./kibana-plugin-public.applicationstart.registermountcontext.md) | Register a context provider for application mounting. Will only be available to applications that depend on the plugin that registered this context. Deprecated, use [CoreSetup.getStartServices()](./kibana-plugin-public.coresetup.getstartservices.md)<!-- -->. |

11 changes: 10 additions & 1 deletion src/core/public/application/application_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -580,14 +580,23 @@ describe('#start()', () => {

it('creates URLs with path parameter', async () => {
service.setup(setupDeps);

const { getUrlForApp } = await service.start(startDeps);

expect(getUrlForApp('app1', { path: 'deep/link' })).toBe('/base-path/app/app1/deep/link');
expect(getUrlForApp('app1', { path: '/deep//link/' })).toBe('/base-path/app/app1/deep/link');
expect(getUrlForApp('app1', { path: '//deep/link//' })).toBe('/base-path/app/app1/deep/link');
expect(getUrlForApp('app1', { path: 'deep/link///' })).toBe('/base-path/app/app1/deep/link');
});

it('creates absolute URLs when `absolute` parameter is true', async () => {
service.setup(setupDeps);
const { getUrlForApp } = await service.start(startDeps);

expect(getUrlForApp('app1', { absolute: true })).toBe('http://localhost/base-path/app/app1');
expect(getUrlForApp('app2', { path: 'deep/link', absolute: true })).toBe(
'http://localhost/base-path/app/app2/deep/link'
);
});
});

describe('navigateToApp', () => {
Expand Down
16 changes: 14 additions & 2 deletions src/core/public/application/application_service.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -272,8 +272,13 @@ export class ApplicationService {
takeUntil(this.stop$)
),
registerMountContext: this.mountContext.registerContext,
getUrlForApp: (appId, { path }: { path?: string } = {}) =>
http.basePath.prepend(getAppUrl(availableMounters, appId, path)),
getUrlForApp: (
appId,
{ path, absolute = false }: { path?: string; absolute?: boolean } = {}
) => {
const relUrl = http.basePath.prepend(getAppUrl(availableMounters, appId, path));
return absolute ? relativeToAbsolute(relUrl) : relUrl;
},
navigateToApp: async (appId, { path, state }: { path?: string; state?: any } = {}) => {
if (await this.shouldNavigate(overlays)) {
this.appLeaveHandlers.delete(this.currentAppId$.value!);
Expand Down Expand Up @@ -364,3 +369,10 @@ const updateStatus = <T extends AppBase>(app: T, statusUpdaters: AppUpdaterWrapp
...changes,
};
};

function relativeToAbsolute(url: string) {
// convert all link urls to absolute urls
const a = document.createElement('a');
a.setAttribute('href', url);
return a.href;
}
Comment on lines +373 to +378
Copy link
Contributor Author

@pgayvallet pgayvallet Feb 10, 2020

Choose a reason for hiding this comment

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

So, this implementation can seems strange, as it relies on actual DOM a node behavior, however

  • This is the same approach that is currently used in NavLinks to creates the absolute url
  • The behavior is specified, this is not relying on internal browser implementation
  • This just works (tm), when other snippets to properly parse and convert a relative url to an absolute one are 'way' more complex, and requires to parse window.location to construct the url.

Still, tell me if we want to change the implementation

10 changes: 8 additions & 2 deletions src/core/public/application/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -593,11 +593,17 @@ export interface ApplicationStart {
navigateToApp(appId: string, options?: { path?: string; state?: any }): Promise<void>;

/**
* Returns a relative URL to a given app, including the global base path.
* Returns an URL to a given app, including the global base path.
* By default, the URL is relative (/basePath/app/my-app).
* Use the `absolute` option to generate an absolute url (http://host:port/basePath/app/my-app)
*
* Note that when generating absolute urls, the protocol, host and port are determined from the browser location.
*
* @param appId
* @param options.path - optional path inside application to deep link to
* @param options.absolute - if true, will returns an absolute url instead of a relative one
*/
getUrlForApp(appId: string, options?: { path?: string }): string;
getUrlForApp(appId: string, options?: { path?: string; absolute?: boolean }): string;

/**
* Register a context provider for application mounting. Will only be available to applications that depend on the
Expand Down
1 change: 1 addition & 0 deletions src/core/public/public.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ export interface ApplicationStart {
capabilities: RecursiveReadonly<Capabilities>;
getUrlForApp(appId: string, options?: {
path?: string;
absolute?: boolean;
}): string;
navigateToApp(appId: string, options?: {
path?: string;
Expand Down