Skip to content
This repository has been archived by the owner on Sep 30, 2024. It is now read-only.

Commit

Permalink
File tree improvements (#9916)
Browse files Browse the repository at this point in the history
- Use a single ?subtree query parameter instead for suggestions and actions.
- Construct the url with the query param in the web directory.
- Do not refresh tree when panel is open
  • Loading branch information
attfarhan authored Apr 17, 2020
1 parent cb96141 commit 0fd04e3
Show file tree
Hide file tree
Showing 8 changed files with 28 additions and 39 deletions.
2 changes: 1 addition & 1 deletion shared/src/components/FileMatchChildren.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ export const FileMatchChildren: React.FunctionComponent<FileMatchProps> = props
className="file-match-children__item-code-wrapper e2e-file-match-children-item-wrapper"
>
<Link
to={`${props.result.file.url}?action${toPositionOrRangeHash({ position })}`}
to={`${props.result.file.url}?subtree=true${toPositionOrRangeHash({ position })}`}
className="file-match-children__item file-match-children__item-clickable e2e-file-match-children-item"
onClick={props.onSelect}
>
Expand Down
2 changes: 1 addition & 1 deletion shared/src/components/RepoFileLink.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export const RepoFileLink: React.FunctionComponent<Props> = ({
return (
<>
<Link to={repoURL}>{repoDisplayName || displayRepoName(repoName)}</Link>{' '}
<Link to={`${fileURL}?action`}>
<Link to={`${fileURL}?subtree=true`}>
{fileBase ? `${fileBase}/` : null}
<strong>{fileName}</strong>
</Link>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ Array [
" ›",
" ",
<a
href="https://example.com/file?action"
href="https://example.com/file?subtree=true"
>
my/
<strong>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ exports[`<HierarchicalLocationsView /> displays a single location when complete
<a
href="/github.com/foo/bar/-/blob/undefined?action"
href="/github.com/foo/bar/-/blob/undefined?subtree=true"
>
<strong>
Expand Down Expand Up @@ -70,7 +70,7 @@ exports[`<HierarchicalLocationsView /> displays a single location when complete
>
<a
className="file-match-children__item file-match-children__item-clickable e2e-file-match-children-item"
href="/github.com/foo/bar/-/blob/undefined?action#L2:1"
href="/github.com/foo/bar/-/blob/undefined?subtree=true#L2:1"
onClick={[Function]}
>
<VisibilitySensor
Expand Down Expand Up @@ -234,7 +234,7 @@ exports[`<HierarchicalLocationsView /> displays multiple locations grouped by fi
<a
href="/github.com/foo/bar/-/blob/file1.txt?action"
href="/github.com/foo/bar/-/blob/file1.txt?subtree=true"
>
<strong>
file1.txt
Expand Down Expand Up @@ -266,7 +266,7 @@ exports[`<HierarchicalLocationsView /> displays multiple locations grouped by fi
>
<a
className="file-match-children__item file-match-children__item-clickable e2e-file-match-children-item"
href="/github.com/foo/bar/-/blob/file1.txt?action#L2:1"
href="/github.com/foo/bar/-/blob/file1.txt?subtree=true#L2:1"
onClick={[Function]}
>
<VisibilitySensor
Expand Down Expand Up @@ -386,7 +386,7 @@ exports[`<HierarchicalLocationsView /> displays partial locations before complet
<a
href="/github.com/foo/bar/-/blob/undefined?action"
href="/github.com/foo/bar/-/blob/undefined?subtree=true"
>
<strong>
Expand Down Expand Up @@ -418,7 +418,7 @@ exports[`<HierarchicalLocationsView /> displays partial locations before complet
>
<a
className="file-match-children__item file-match-children__item-clickable e2e-file-match-children-item"
href="/github.com/foo/bar/-/blob/undefined?action#L2:1"
href="/github.com/foo/bar/-/blob/undefined?subtree=true#L2:1"
onClick={[Function]}
>
<VisibilitySensor
Expand Down
19 changes: 6 additions & 13 deletions shared/src/util/url.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import { isEmpty } from 'lodash'
import { parseSearchQuery, CharacterRange } from '../search/parser/parser'
import { replaceRange } from './strings'
import { discreteValueAliases } from '../search/parser/filters'
import { URLToFileContext } from '../platform/context'

export interface RepoSpec {
/**
Expand Down Expand Up @@ -420,11 +419,11 @@ function parseLineOrPositionOrRange(lineChar: string): LineOrPositionOrRange {
return lpr
}

function toRenderModeAndActionQuery(ctx: Partial<RenderModeSpec>, isWebUrl?: boolean): string {
function toRenderModeQuery(ctx: Partial<RenderModeSpec>): string {
if (ctx.renderMode === 'code') {
return isWebUrl ? '?action&view=code' : '?view=code'
return '?view=code'
}
return isWebUrl ? '?action' : ''
return ''
}

/**
Expand Down Expand Up @@ -472,16 +471,10 @@ export function encodeRepoRev(repo: string, rev?: string): string {
}

export function toPrettyBlobURL(
target: RepoFile &
Partial<UIPositionSpec> &
Partial<ViewStateSpec> &
Partial<UIRangeSpec> &
Partial<RenderModeSpec>,
context?: Partial<URLToFileContext>
target: RepoFile & Partial<UIPositionSpec> & Partial<ViewStateSpec> & Partial<UIRangeSpec> & Partial<RenderModeSpec>
): string {
return `/${encodeRepoRev(target.repoName, target.rev)}/-/blob/${target.filePath}${toRenderModeAndActionQuery(
target,
context?.isWebURL
return `/${encodeRepoRev(target.repoName, target.rev)}/-/blob/${target.filePath}${toRenderModeQuery(
target
)}${toPositionOrRangeHash(target)}${toViewStateHashComponent(target.viewState)}`
}

Expand Down
4 changes: 3 additions & 1 deletion web/src/platform/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,9 @@ export function createPlatformContext(): PlatformContext {
function toPrettyWebBlobURL(
ctx: RepoFile & Partial<UIPositionSpec> & Partial<ViewStateSpec> & Partial<UIRangeSpec> & Partial<RenderModeSpec>
): string {
return toPrettyBlobURL(ctx, { isWebURL: true })
const url = new URL(toPrettyBlobURL(ctx), location.href)
url.searchParams.set('subtree', 'true')
return url.pathname + url.search + url.hash
}

const settingsCascadeFragment = gql`
Expand Down
4 changes: 2 additions & 2 deletions web/src/search/input/Suggestion.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ export function createSuggestion(item: GQL.SearchSuggestion): Suggestion | undef
type: NonFilterSuggestionType.dir,
value: '^' + escapeRegExp(item.path),
description: descriptionParts.join(' — '),
url: `${item.url}?suggestion`,
url: `${item.url}?subtree=true`,
label: 'go to dir',
}
}
Expand All @@ -105,7 +105,7 @@ export function createSuggestion(item: GQL.SearchSuggestion): Suggestion | undef
value: formatRegExp(item.path),
displayValue: item.name,
description: descriptionParts.join(' — '),
url: `${item.url}?suggestion`,
url: `${item.url}?subtree=true`,
label: 'go to file',
}
}
Expand Down
22 changes: 8 additions & 14 deletions web/src/tree/Tree.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -243,12 +243,12 @@ export class Tree extends React.PureComponent<Props, State> {
.subscribe((props: Props) => {
const newParentPath = props.activePathIsDir ? props.activePath : dirname(props.activePath)
const queryParams = new URLSearchParams(this.props.history.location.search)
// If we're updating due to a file or directory suggestion, load the relevant partial tree and jump to the file.
const queryParamsHasSubtree = queryParams.get('subtree') === 'true'

// If we're updating due to a file/directory suggestion or code intel action,
// load the relevant partial tree and jump to the file.
// This case is only used when going from an ancestor to a child file/directory, or equal.
if (
(queryParams.has('suggestion') || queryParams.has('action')) &&
dotPathAsUndefined(newParentPath)
) {
if (queryParamsHasSubtree && !queryParams.has('tab') && dotPathAsUndefined(newParentPath)) {
this.setState({
parentPath: dotPathAsUndefined(newParentPath),
resolveTo: [newParentPath],
Expand All @@ -268,15 +268,9 @@ export class Tree extends React.PureComponent<Props, State> {
})
}

// Strip the ?suggestion query param. Handle both when going from ancestor -> child and child -> ancestor.
if (queryParams.has('suggestion')) {
queryParams.delete('suggestion')
this.props.history.replace({
search: queryParams.toString(),
hash: this.props.history.location.hash,
})
} else if (queryParams.has('action')) {
queryParams.delete('action')
// Strip the ?subtree query param. Handle both when going from ancestor -> child and child -> ancestor.
queryParams.delete('subtree')
if (queryParamsHasSubtree && !queryParams.has('tab')) {
this.props.history.replace({
search: queryParams.toString(),
hash: this.props.history.location.hash,
Expand Down

0 comments on commit 0fd04e3

Please sign in to comment.