Display owned files as links to the diff Heavily inspired (in terms of implementation and styles) on the Gerrit's `Files` tab: * filename (with extension) is a link to the file's diff * when group of files form a given dir are rendered the first one is highlighted so that it is easier to spot when dir changes * fonts, colors, hover etc are consistent with `Files` tab NOTE: The patchset selector is going to be addressed in a follow-up change Bug: Issue 376837413 Change-Id: I6e464235dca741d4d896e7875a7dc5179eb119f1
diff --git a/owners/web/gerrit-model.ts b/owners/web/gerrit-model.ts index f2b88bf..da0d42c 100644 --- a/owners/web/gerrit-model.ts +++ b/owners/web/gerrit-model.ts
@@ -35,3 +35,20 @@ export interface GrAccountLabel extends Element { getAccountsModel: Provider<AccountsModel>; } + +/** + * Partial Gerrit's `SpecialFilePath` enum + */ +export enum SpecialFilePath { + COMMIT_MESSAGE = '/COMMIT_MSG', + MERGE_LIST = '/MERGE_LIST', +} + +/** + * Partial Gerrit's `Window` interface defintion so that `getBaseUrl` function can work. + */ +declare global { + interface Window { + CANONICAL_PATH?: string; + } +}
diff --git a/owners/web/gr-owned-files.ts b/owners/web/gr-owned-files.ts index b329d42..969d91c 100644 --- a/owners/web/gr-owned-files.ts +++ b/owners/web/gr-owned-files.ts
@@ -15,7 +15,8 @@ * limitations under the License. */ -import {html, LitElement, PropertyValues, nothing, CSSResult} from 'lit'; +import {css, html, LitElement, PropertyValues, nothing, CSSResult} from 'lit'; +import {ifDefined} from 'lit/directives/if-defined.js'; import {OwnersMixin} from './owners-mixin'; import {customElement, property} from 'lit/decorators'; import { @@ -27,6 +28,13 @@ } from '@gerritcodereview/typescript-api/rest-api'; import {User, UserRole} from './owners-model'; import {isOwner, OwnedFiles, OWNERS_SUBMIT_REQUIREMENT} from './owners-service'; +import { + computeDisplayPath, + diffFilePaths, + encodeURL, + getBaseUrl, + truncatePath, +} from './utils'; const common = OwnersMixin(LitElement); @@ -49,7 +57,7 @@ ); } - protected static commonStyles(): CSSResult[] { + static commonStyles(): CSSResult[] { return [window?.Gerrit?.styles.font as CSSResult]; } @@ -71,18 +79,179 @@ } } +@customElement('gr-owned-files-list') +export class GrOwnedFilesList extends LitElement { + @property({type: Object}) + change?: ChangeInfo; + + @property({type: Object}) + revision?: RevisionInfo; + + @property({type: Array}) + ownedFiles?: string[]; + + static override get styles() { + return [ + ...OwnedFilesCommon.commonStyles(), + css` + :host { + display: block; + } + .row { + align-items: center; + border-top: 1px solid var(--border-color); + display: flex; + min-height: calc(var(--line-height-normal) + 2 * var(--spacing-s)); + padding: var(--spacing-xs) var(--spacing-l); + } + .header-row { + background-color: var(--background-color-secondary); + } + .file-row { + cursor: pointer; + } + .file-row:hover { + background-color: var(--hover-background-color); + } + .file-row.selected { + background-color: var(--selection-background-color); + } + .path { + cursor: pointer; + flex: 1; + /* Wrap it into multiple lines if too long. */ + white-space: normal; + word-break: break-word; + } + .matchingFilePath { + color: var(--deemphasized-text-color); + } + .newFilePath { + color: var(--primary-text-color); + } + .fileName { + color: var(--link-color); + } + .truncatedFileName { + display: none; + } + .row:focus { + outline: none; + } + .row:hover { + opacity: 100; + } + .pathLink { + display: inline-block; + margin: -2px 0; + padding: var(--spacing-s) 0; + text-decoration: none; + } + .pathLink:hover span.fullFileName, + .pathLink:hover span.truncatedFileName { + text-decoration: underline; + } + `, + ]; + } + + override render() { + if (!this.change || !this.revision || !this.ownedFiles) return nothing; + + return html` + <div id="container" role="grid" aria-label="Owned files list"> + ${this.renderOwnedFilesHeaderRow()} + ${this.ownedFiles?.map((ownedFile, index) => + this.renderOwnedFileRow(ownedFile, index) + )} + </div> + `; + } + + private renderOwnedFilesHeaderRow() { + return html` + <div class="header-row row" role="row"> + <div class="path" role="columnheader">File</div> + </div> + `; + } + + private renderOwnedFileRow(ownedFile: string, index: number) { + return html` + <div + class="file-row row" + tabindex="-1" + role="row" + aria-label=${ownedFile} + > + ${this.renderFilePath(ownedFile, index)} + </div> + `; + } + + private renderFilePath(file: string, index: number) { + const displayPath = computeDisplayPath(file); + const previousFile = (this.ownedFiles ?? [])[index - 1]; + return html` + <span class="path" role="gridcell"> + <a + class="pathLink" + href=${ifDefined(computeDiffUrl(file, this.change, this.revision))} + > + <span title=${displayPath} class="fullFileName"> + ${this.renderStyledPath(file, previousFile)} + </span> + <span title=${displayPath} class="truncatedFileName"> + ${truncatePath(displayPath)} + </span> + </a> + </span> + `; + } + + private renderStyledPath(filePath: string, previousFilePath?: string) { + const {matchingFolders, newFolders, fileName} = diffFilePaths( + filePath, + previousFilePath + ); + return [ + matchingFolders.length > 0 + ? html`<span class="matchingFilePath">${matchingFolders}</span>` + : nothing, + newFolders.length > 0 + ? html`<span class="newFilePath">${newFolders}</span>` + : nothing, + html`<span class="fileName">${fileName}</span>`, + ]; + } +} + export const OWNED_FILES_TAB_CONTENT = 'owned-files-tab-content'; @customElement(OWNED_FILES_TAB_CONTENT) export class OwnedFilesTabContent extends OwnedFilesCommon { static override get styles() { - return [...OwnedFilesCommon.commonStyles()]; + return [ + ...OwnedFilesCommon.commonStyles(), + css` + :host { + display: block; + } + `, + ]; } override render() { - if (this.hidden || !this.ownedFiles) return nothing; - return html`<div> - ${this.ownedFiles.map(ownedFile => html`<div>${ownedFile}</div>`)} - </div>`; + if (this.hidden) return nothing; + + return html` + <gr-owned-files-list + id="ownedFilesList" + .change=${this.change} + .revision=${this.revision} + .ownedFiles=${this.ownedFiles} + > + </gr-owned-files-list> + `; } } @@ -140,3 +309,23 @@ return ownedFiles; } + +export function computeDiffUrl( + file: string, + change?: ChangeInfo, + revision?: RevisionInfo +) { + if (change === undefined || revision?._number === undefined) { + return; + } + + let repo = ''; + if (change.project) repo = `${encodeURL(change.project)}/+/`; + + // TODO it can be a range of patchsets but `PatchRange` is not passed to the `change-view-tab-content` :/ fix in Gerrit? + const range = `/${revision._number}`; + + const path = `/${encodeURL(file)}`; + + return `${getBaseUrl()}/c/${repo}${change._number}${range}${path}`; +}
diff --git a/owners/web/tsconfig.json b/owners/web/tsconfig.json index c4f9468..0a15ca9 100644 --- a/owners/web/tsconfig.json +++ b/owners/web/tsconfig.json
@@ -2,8 +2,14 @@ "extends": "../../tsconfig-plugins-base.json", "compilerOptions": { "outDir": "../../../.ts-out/plugins/owners", /* overridden by bazel */ + "lib": [ + "dom", + "dom.iterable", + "es2021", + "webworker" + ], }, "include": [ "**/*.ts" - ] + ], }
diff --git a/owners/web/utils.ts b/owners/web/utils.ts index d3197f1..81e0adb 100644 --- a/owners/web/utils.ts +++ b/owners/web/utils.ts
@@ -15,6 +15,8 @@ * limitations under the License. */ +import {SpecialFilePath} from './gerrit-model'; + export function deepEqual<T>(a: T, b: T): boolean { if (a === b) return true; if (a === undefined || b === undefined) return false; @@ -69,3 +71,152 @@ } return el.querySelector<E>(selector) ?? undefined; } + +/** + * Copied from Gerrit's path-list-util.ts. + */ +export function computeDisplayPath(path?: string) { + if (path === SpecialFilePath.COMMIT_MESSAGE) { + return 'Commit message'; + } else if (path === SpecialFilePath.MERGE_LIST) { + return 'Merge list'; + } + return path ?? ''; +} + +/** + * Separates a path into: + * - The part that matches another path, + * - The part that does not match the other path, + * - The file name + * + * For example: + * diffFilePaths('same/part/new/part/foo.js', 'same/part/different/foo.js'); + * yields: { + * matchingFolders: 'same/part/', + * newFolders: 'new/part/', + * fileName: 'foo.js', + * } + * + * Copied from Gerrit's string-util.ts. + */ +export function diffFilePaths(filePath: string, otherFilePath?: string) { + // Separate each string into an array of folder names + file name. + const displayPath = computeDisplayPath(filePath); + const previousFileDisplayPath = computeDisplayPath(otherFilePath); + const displayPathParts = displayPath.split('/'); + const previousFileDisplayPathParts = previousFileDisplayPath.split('/'); + + // Construct separate strings for matching folders, new folders, and file + // name. + const firstDifferencePartIndex = displayPathParts.findIndex( + (part, index) => previousFileDisplayPathParts[index] !== part + ); + const matchingSection = displayPathParts + .slice(0, firstDifferencePartIndex) + .join('/'); + const newFolderSection = displayPathParts + .slice(firstDifferencePartIndex, -1) + .join('/'); + const fileNameSection = displayPathParts[displayPathParts.length - 1]; + + // Note: folder sections need '/' appended back. + return { + matchingFolders: matchingSection.length > 0 ? `${matchingSection}/` : '', + newFolders: newFolderSection.length > 0 ? `${newFolderSection}/` : '', + fileName: fileNameSection, + }; +} + +/** + * Truncates URLs to display filename only + * Example + * // returns '.../text.html' + * util.truncatePath.('dir/text.html'); + * Example + * // returns 'text.html' + * util.truncatePath.('text.html'); + * + * Copied from Gerrit's path-list-util.ts. + */ +export function truncatePath(path: string, threshold = 1) { + const pathPieces = path.split('/'); + + if (pathPieces.length <= threshold) { + return path; + } + + const index = pathPieces.length - threshold; + // Character is an ellipsis. + return `\u2026/${pathPieces.slice(index).join('/')}`; +} + +/** + * Encodes *parts* of a URL. See inline comments below for the details. + * Note specifically that ? & = # are encoded. So this is very close to + * encodeURIComponent() with some tweaks. + * + * Copied from Gerrit's url-util.ts. + */ +export function encodeURL(url: string): string { + // gr-page decodes the entire URL, and then decodes once more the + // individual regex matching groups. It uses `decodeURIComponent()`, which + // will choke on singular `%` chars without two trailing digits. We prefer + // to not double encode *everything* (just for readaiblity and simplicity), + // but `%` *must* be double encoded. + let output = url.replaceAll('%', '%25'); // `replaceAll` function requires `es2021` hence the `lib` section was added to `tsconfig.json` + // `+` also requires double encoding, because `%2B` would be decoded to `+` + // and then replaced by ` `. + output = output.replaceAll('+', '%2B'); + + // This escapes ALL characters EXCEPT: + // A–Z a–z 0–9 - _ . ! ~ * ' ( ) + // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/encodeURIComponent + output = encodeURIComponent(output); + + // If we would use `encodeURI()` instead of `encodeURIComponent()`, then we + // would also NOT encode: + // ; / ? : @ & = + $ , # + // + // That would be more readable, but for example ? and & have special meaning + // in the URL, so they must be encoded. Let's discuss all these chars and + // decide whether we have to encode them or not. + // + // ? & = # have to be encoded. Otherwise we might mess up the URL. + // + // : @ do not have to be encoded, because we are only dealing with path, + // query and fragment of the URL, not with scheme, user, host, port. + // For search queries it is much nicer to not encode those chars, think of + // searching for `owner:spearce@spearce.org`. + // + // / does not have to be encoded, because we don't care about individual path + // components. File path and repo names are so much nicer to read without / + // being encoded! + // + // + must be encoded, because we want to use it instead of %20 for spaces, see + // below. + // + // ; $ , probably don't have to be encoded, but we don't bother about them + // much, so we don't reverse the encoding here, but we don't think it would + // cause any harm, if we did. + output = output.replace(/%3A/g, ':'); + output = output.replace(/%40/g, '@'); + output = output.replace(/%2F/g, '/'); + + // gr-page replaces `+` by ` ` in addition to calling `decodeURIComponent()`. + // So we can use `+` to increase readability. + output = output.replace(/%20/g, '+'); + + return output; +} + +/** + * Function to obtain the base URL. + * + * Copied from Gerrit's url-util.ts. + */ +export function getBaseUrl(): string { + // window is not defined in service worker, therefore no CANONICAL_PATH + if (typeof window === 'undefined') return ''; + return self.CANONICAL_PATH || ''; +}