Skip to content

Commit

Permalink
fix ScopedHistory.createHref to prepend location with scoped history …
Browse files Browse the repository at this point in the history
…basePath (elastic#62407)

* fix createHref to prepend with scoped history basePath + add option to exclude it.

* fix prependBasePath behavior

* fix test plugins urls

* add pathname to endpoint url builder methods

* Revert "add pathname to endpoint url builder methods"

This reverts commit 7604932

* adapt createHref instead of prependBasePath

* use object options for createHref

* update generated doc
  • Loading branch information
pgayvallet authored and Maja Grubic committed Apr 16, 2020
1 parent 96e4749 commit c63933a
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@

## ScopedHistory.createHref property

Creates an href (string) to the location.
Creates an href (string) to the location. If `prependBasePath` is true (default), it will prepend the location's path with the scoped history basePath.

<b>Signature:</b>

```typescript
createHref: (location: LocationDescriptorObject<HistoryLocationState>) => string;
createHref: (location: LocationDescriptorObject<HistoryLocationState>, { prependBasePath }?: {
prependBasePath?: boolean | undefined;
}) => string;
```
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export declare class ScopedHistory<HistoryLocationState = unknown> implements Hi
| --- | --- | --- | --- |
| [action](./kibana-plugin-core-public.scopedhistory.action.md) | | <code>Action</code> | The last action dispatched on the history stack. |
| [block](./kibana-plugin-core-public.scopedhistory.block.md) | | <code>(prompt?: string &#124; boolean &#124; History.TransitionPromptHook&lt;HistoryLocationState&gt; &#124; undefined) =&gt; UnregisterCallback</code> | Not supported. Use [AppMountParameters.onAppLeave](./kibana-plugin-core-public.appmountparameters.onappleave.md)<!-- -->. |
| [createHref](./kibana-plugin-core-public.scopedhistory.createhref.md) | | <code>(location: LocationDescriptorObject&lt;HistoryLocationState&gt;) =&gt; string</code> | Creates an href (string) to the location. |
| [createHref](./kibana-plugin-core-public.scopedhistory.createhref.md) | | <code>(location: LocationDescriptorObject&lt;HistoryLocationState&gt;, { prependBasePath }?: {</code><br/><code> prependBasePath?: boolean &#124; undefined;</code><br/><code> }) =&gt; string</code> | Creates an href (string) to the location. If <code>prependBasePath</code> is true (default), it will prepend the location's path with the scoped history basePath. |
| [createSubHistory](./kibana-plugin-core-public.scopedhistory.createsubhistory.md) | | <code>&lt;SubHistoryLocationState = unknown&gt;(basePath: string) =&gt; ScopedHistory&lt;SubHistoryLocationState&gt;</code> | Creates a <code>ScopedHistory</code> for a subpath of this <code>ScopedHistory</code>. Useful for applications that may have sub-apps that do not need access to the containing application's history. |
| [go](./kibana-plugin-core-public.scopedhistory.go.md) | | <code>(n: number) =&gt; void</code> | Send the user forward or backwards in the history stack. |
| [goBack](./kibana-plugin-core-public.scopedhistory.goback.md) | | <code>() =&gt; void</code> | Send the user one location back in the history stack. Equivalent to calling [ScopedHistory.go(-1)](./kibana-plugin-core-public.scopedhistory.go.md)<!-- -->. If no more entries are available backwards, this is a no-op. |
Expand Down
25 changes: 23 additions & 2 deletions src/core/public/application/scoped_history.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -268,11 +268,32 @@ describe('ScopedHistory', () => {
const gh = createMemoryHistory();
gh.push('/app/wow');
const h = new ScopedHistory(gh, '/app/wow');
expect(h.createHref({ pathname: '' })).toEqual(`/`);
expect(h.createHref({ pathname: '' })).toEqual(`/app/wow`);
expect(h.createHref({})).toEqual(`/app/wow`);
expect(h.createHref({ pathname: '/new-page', search: '?alpha=true' })).toEqual(
`/new-page?alpha=true`
`/app/wow/new-page?alpha=true`
);
});

it('behave correctly with slash-ending basePath', () => {
const gh = createMemoryHistory();
gh.push('/app/wow/');
const h = new ScopedHistory(gh, '/app/wow/');
expect(h.createHref({ pathname: '' })).toEqual(`/app/wow/`);
expect(h.createHref({ pathname: '/new-page', search: '?alpha=true' })).toEqual(
`/app/wow/new-page?alpha=true`
);
});

it('skips the scoped history path when `prependBasePath` is false', () => {
const gh = createMemoryHistory();
gh.push('/app/wow');
const h = new ScopedHistory(gh, '/app/wow');
expect(h.createHref({ pathname: '' }, { prependBasePath: false })).toEqual(`/`);
expect(
h.createHref({ pathname: '/new-page', search: '?alpha=true' }, { prependBasePath: false })
).toEqual(`/new-page?alpha=true`);
});
});

describe('action', () => {
Expand Down
20 changes: 17 additions & 3 deletions src/core/public/application/scoped_history.ts
Original file line number Diff line number Diff line change
Expand Up @@ -219,11 +219,26 @@ export class ScopedHistory<HistoryLocationState = unknown>

/**
* Creates an href (string) to the location.
* If `prependBasePath` is true (default), it will prepend the location's path with the scoped history basePath.
*
* @param location
* @param prependBasePath
*/
public createHref = (location: LocationDescriptorObject<HistoryLocationState>): Href => {
public createHref = (
location: LocationDescriptorObject<HistoryLocationState>,
{ prependBasePath = true }: { prependBasePath?: boolean } = {}
): Href => {
this.verifyActive();
if (prependBasePath) {
location = this.prependBasePath(location);
if (location.pathname === undefined) {
// we always want to create an url relative to the basePath
// so if pathname is not present, we use the history's basePath as default
// we are doing that here because `prependBasePath` should not
// alter pathname for other method calls
location.pathname = this.basePath;
}
}
return this.parentHistory.createHref(location);
};

Expand Down Expand Up @@ -254,8 +269,7 @@ export class ScopedHistory<HistoryLocationState = unknown>
* Prepends the base path to string.
*/
private prependBasePathToString(path: string): string {
path = path.startsWith('/') ? path.slice(1) : path;
return path.length ? `${this.basePath}/${path}` : this.basePath;
return path.length ? `${this.basePath}/${path}`.replace(/\/{2,}/g, '/') : this.basePath;
}

/**
Expand Down
4 changes: 3 additions & 1 deletion src/core/public/public.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -1196,7 +1196,9 @@ export class ScopedHistory<HistoryLocationState = unknown> implements History<Hi
constructor(parentHistory: History, basePath: string);
get action(): Action;
block: (prompt?: string | boolean | History.TransitionPromptHook<HistoryLocationState> | undefined) => UnregisterCallback;
createHref: (location: LocationDescriptorObject<HistoryLocationState>) => string;
createHref: (location: LocationDescriptorObject<HistoryLocationState>, { prependBasePath }?: {
prependBasePath?: boolean | undefined;
}) => string;
createSubHistory: <SubHistoryLocationState = unknown>(basePath: string) => ScopedHistory<SubHistoryLocationState>;
go: (n: number) => void;
goBack: () => void;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ const Nav = withRouter(({ history, navigateToApp }: NavProps) => (
{
id: 'home',
name: 'Home',
onClick: () => history.push('/'),
onClick: () => history.push(''),
'data-test-subj': 'fooNavHome',
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ const Nav = withRouter(({ history, navigateToApp }: NavProps) => (
{
id: 'home',
name: 'Home',
onClick: () => navigateToApp('bar', { path: '/' }),
onClick: () => navigateToApp('bar', { path: '' }),
'data-test-subj': 'barNavHome',
},
{
Expand Down

0 comments on commit c63933a

Please sign in to comment.