Skip to content

Commit

Permalink
Add types to various low-level functions (#31781)
Browse files Browse the repository at this point in the history
Adds types to various low-level modules. All changes are type-only, no
runtime changes. `tsc` now reports 38 less errors.

One problem was that `@types/sortablejs` does not accept promise return
in its functions which triggered the linter, so I disabled the rules on
those line.
  • Loading branch information
silverwind authored Aug 10, 2024
1 parent 9633f33 commit 32075d2
Show file tree
Hide file tree
Showing 13 changed files with 123 additions and 77 deletions.
22 changes: 18 additions & 4 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@
"stylelint-declaration-strict-value": "1.10.6",
"stylelint-value-no-unknown-custom-properties": "6.0.1",
"svgo": "3.3.2",
"type-fest": "4.23.0",
"updates": "16.3.7",
"vite-string-plugin": "1.3.4",
"vitest": "2.0.5"
Expand Down
9 changes: 9 additions & 0 deletions types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@ declare module '*.svg' {
export default value;
}

declare module '*.css' {
const value: string;
export default value;
}

declare let __webpack_public_path__: string;

interface Window {
Expand All @@ -20,3 +25,7 @@ declare module 'htmx.org/dist/htmx.esm.js' {
const value = await import('htmx.org');
export default value;
}

interface Element {
_tippy: import('tippy.js').Instance;
}
2 changes: 1 addition & 1 deletion web_src/js/features/dropzone.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ function addCopyLink(file) {
copyLinkEl.addEventListener('click', async (e) => {
e.preventDefault();
const success = await clippie(generateMarkdownLinkForAttachment(file));
showTemporaryTooltip(e.target, success ? i18n.copy_success : i18n.copy_error);
showTemporaryTooltip(e.target as Element, success ? i18n.copy_success : i18n.copy_error);
});
file.previewTemplate.append(copyLinkEl);
}
Expand Down
2 changes: 1 addition & 1 deletion web_src/js/features/repo-issue-list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ async function initIssuePinSort() {

createSortable(pinDiv, {
group: 'shared',
onEnd: pinMoveEnd,
onEnd: pinMoveEnd, // eslint-disable-line @typescript-eslint/no-misused-promises
});
}

Expand Down
6 changes: 3 additions & 3 deletions web_src/js/features/repo-projects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ async function initRepoProjectSortable() {
handle: '.project-column-header',
delayOnTouchOnly: true,
delay: 500,
onSort: async () => {
onSort: async () => { // eslint-disable-line @typescript-eslint/no-misused-promises
boardColumns = mainBoard.querySelectorAll('.project-column');

const columnSorting = {
Expand All @@ -84,8 +84,8 @@ async function initRepoProjectSortable() {
const boardCardList = boardColumn.querySelectorAll('.cards')[0];
createSortable(boardCardList, {
group: 'shared',
onAdd: moveIssue,
onUpdate: moveIssue,
onAdd: moveIssue, // eslint-disable-line @typescript-eslint/no-misused-promises
onUpdate: moveIssue, // eslint-disable-line @typescript-eslint/no-misused-promises
delayOnTouchOnly: true,
delay: 500,
});
Expand Down
2 changes: 1 addition & 1 deletion web_src/js/features/stopwatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export function initStopwatch() {
stopwatchEl.removeAttribute('href'); // intended for noscript mode only

createTippy(stopwatchEl, {
content: stopwatchPopup.cloneNode(true),
content: stopwatchPopup.cloneNode(true) as Element,
placement: 'bottom-end',
trigger: 'click',
maxWidth: 'none',
Expand Down
14 changes: 9 additions & 5 deletions web_src/js/modules/dirauto.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import {isDocumentFragmentOrElementNode} from '../utils/dom.ts';

type DirElement = HTMLInputElement | HTMLTextAreaElement;

// for performance considerations, it only uses performant syntax
function attachDirAuto(el) {
function attachDirAuto(el: DirElement) {
if (el.type !== 'hidden' &&
el.type !== 'checkbox' &&
el.type !== 'radio' &&
Expand All @@ -18,10 +20,12 @@ export function initDirAuto() {
const mutation = mutationList[i];
const len = mutation.addedNodes.length;
for (let i = 0; i < len; i++) {
const addedNode = mutation.addedNodes[i];
const addedNode = mutation.addedNodes[i] as HTMLElement;
if (!isDocumentFragmentOrElementNode(addedNode)) continue;
if (addedNode.nodeName === 'INPUT' || addedNode.nodeName === 'TEXTAREA') attachDirAuto(addedNode);
const children = addedNode.querySelectorAll('input, textarea');
if (addedNode.nodeName === 'INPUT' || addedNode.nodeName === 'TEXTAREA') {
attachDirAuto(addedNode as DirElement);
}
const children = addedNode.querySelectorAll<DirElement>('input, textarea');
const len = children.length;
for (let childIdx = 0; childIdx < len; childIdx++) {
attachDirAuto(children[childIdx]);
Expand All @@ -30,7 +34,7 @@ export function initDirAuto() {
}
});

const docNodes = document.querySelectorAll('input, textarea');
const docNodes = document.querySelectorAll<DirElement>('input, textarea');
const len = docNodes.length;
for (let i = 0; i < len; i++) {
attachDirAuto(docNodes[i]);
Expand Down
6 changes: 4 additions & 2 deletions web_src/js/modules/sortable.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
export async function createSortable(el, opts = {}) {
import type {SortableOptions} from 'sortablejs';

export async function createSortable(el, opts: {handle?: string} & SortableOptions = {}) {
const {Sortable} = await import(/* webpackChunkName: "sortablejs" */'sortablejs');

return new Sortable(el, {
Expand All @@ -15,5 +17,5 @@ export async function createSortable(el, opts = {}) {
opts.onUnchoose?.(e);
},
...opts,
});
} satisfies SortableOptions);
}
3 changes: 2 additions & 1 deletion web_src/js/modules/stores.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {reactive} from 'vue';
import type {Reactive} from 'vue';

let diffTreeStoreReactive;
let diffTreeStoreReactive: Reactive<Record<string, any>>;
export function diffTreeStore() {
if (!diffTreeStoreReactive) {
diffTreeStoreReactive = reactive(window.config.pageData.diffFileInfo);
Expand Down
47 changes: 24 additions & 23 deletions web_src/js/modules/tippy.ts
Original file line number Diff line number Diff line change
@@ -1,32 +1,38 @@
import tippy, {followCursor} from 'tippy.js';
import {isDocumentFragmentOrElementNode} from '../utils/dom.ts';
import {formatDatetime} from '../utils/time.ts';
import type {Content, Instance, Props} from 'tippy.js';

const visibleInstances = new Set();
type TippyOpts = {
role?: string,
theme?: 'default' | 'tooltip' | 'menu' | 'box-with-header' | 'bare',
} & Partial<Props>;

const visibleInstances = new Set<Instance>();
const arrowSvg = `<svg width="16" height="7"><path d="m0 7 8-7 8 7Z" class="tippy-svg-arrow-outer"/><path d="m0 8 8-7 8 7Z" class="tippy-svg-arrow-inner"/></svg>`;

export function createTippy(target, opts = {}) {
export function createTippy(target: Element, opts: TippyOpts = {}) {
// the callback functions should be destructured from opts,
// because we should use our own wrapper functions to handle them, do not let the user override them
const {onHide, onShow, onDestroy, role, theme, arrow, ...other} = opts;

const instance = tippy(target, {
const instance: Instance = tippy(target, {
appendTo: document.body,
animation: false,
allowHTML: false,
hideOnClick: false,
interactiveBorder: 20,
ignoreAttributes: true,
maxWidth: 500, // increase over default 350px
onHide: (instance) => {
onHide: (instance: Instance) => {
visibleInstances.delete(instance);
return onHide?.(instance);
},
onDestroy: (instance) => {
onDestroy: (instance: Instance) => {
visibleInstances.delete(instance);
return onDestroy?.(instance);
},
onShow: (instance) => {
onShow: (instance: Instance) => {
// hide other tooltip instances so only one tooltip shows at a time
for (const visibleInstance of visibleInstances) {
if (visibleInstance.props.role === 'tooltip') {
Expand All @@ -43,7 +49,7 @@ export function createTippy(target, opts = {}) {
theme: theme || role || 'default',
plugins: [followCursor],
...other,
});
} satisfies Partial<Props>);

if (role === 'menu') {
target.setAttribute('aria-haspopup', 'true');
Expand All @@ -58,12 +64,8 @@ export function createTippy(target, opts = {}) {
* If the target element has no content, then no tooltip will be attached, and it returns null.
*
* Note: "tooltip" doesn't equal to "tippy". "tooltip" means a auto-popup content, it just uses tippy as the implementation.
*
* @param target {HTMLElement}
* @param content {null|string}
* @returns {null|tippy}
*/
function attachTooltip(target, content = null) {
function attachTooltip(target: Element, content: Content = null) {
switchTitleToTooltip(target);

content = content ?? target.getAttribute('data-tooltip-content');
Expand All @@ -84,7 +86,7 @@ function attachTooltip(target, content = null) {
placement: target.getAttribute('data-tooltip-placement') || 'top-start',
followCursor: target.getAttribute('data-tooltip-follow-cursor') || false,
...(target.getAttribute('data-tooltip-interactive') === 'true' ? {interactive: true, aria: {content: 'describedby', expanded: false}} : {}),
};
} as TippyOpts;

if (!target._tippy) {
createTippy(target, props);
Expand All @@ -94,7 +96,7 @@ function attachTooltip(target, content = null) {
return target._tippy;
}

function switchTitleToTooltip(target) {
function switchTitleToTooltip(target: Element) {
let title = target.getAttribute('title');
if (title) {
// apply custom formatting to relative-time's tooltips
Expand All @@ -118,16 +120,15 @@ function switchTitleToTooltip(target) {
* According to https://www.w3.org/TR/DOM-Level-3-Events/#events-mouseevent-event-order , mouseover event is fired before mouseenter event
* Some browsers like PaleMoon don't support "addEventListener('mouseenter', capture)"
* The tippy by default uses "mouseenter" event to show, so we use "mouseover" event to switch to tippy
* @param e {Event}
*/
function lazyTooltipOnMouseHover(e) {
function lazyTooltipOnMouseHover(e: MouseEvent) {
e.target.removeEventListener('mouseover', lazyTooltipOnMouseHover, true);
attachTooltip(this);
}

// Activate the tooltip for current element.
// If the element has no aria-label, use the tooltip content as aria-label.
function attachLazyTooltip(el) {
function attachLazyTooltip(el: Element) {
el.addEventListener('mouseover', lazyTooltipOnMouseHover, {capture: true});

// meanwhile, if the element has no aria-label, use the tooltip content as aria-label
Expand All @@ -140,15 +141,15 @@ function attachLazyTooltip(el) {
}

// Activate the tooltip for all children elements.
function attachChildrenLazyTooltip(target) {
for (const el of target.querySelectorAll('[data-tooltip-content]')) {
function attachChildrenLazyTooltip(target: Element) {
for (const el of target.querySelectorAll<Element>('[data-tooltip-content]')) {
attachLazyTooltip(el);
}
}

export function initGlobalTooltips() {
// use MutationObserver to detect new "data-tooltip-content" elements added to the DOM, or attributes changed
const observerConnect = (observer) => observer.observe(document, {
const observerConnect = (observer: MutationObserver) => observer.observe(document, {
subtree: true,
childList: true,
attributeFilter: ['data-tooltip-content', 'title'],
Expand All @@ -159,15 +160,15 @@ export function initGlobalTooltips() {
for (const mutation of [...mutationList, ...pending]) {
if (mutation.type === 'childList') {
// mainly for Vue components and AJAX rendered elements
for (const el of mutation.addedNodes) {
for (const el of mutation.addedNodes as NodeListOf<Element>) {
if (!isDocumentFragmentOrElementNode(el)) continue;
attachChildrenLazyTooltip(el);
if (el.hasAttribute('data-tooltip-content')) {
attachLazyTooltip(el);
}
}
} else if (mutation.type === 'attributes') {
attachTooltip(mutation.target);
attachTooltip(mutation.target as Element);
}
}
observerConnect(observer);
Expand All @@ -177,7 +178,7 @@ export function initGlobalTooltips() {
attachChildrenLazyTooltip(document.documentElement);
}

export function showTemporaryTooltip(target, content) {
export function showTemporaryTooltip(target: Element, content: Content) {
// if the target is inside a dropdown, don't show the tooltip because when the dropdown
// closes, the tippy would be pushed unsightly to the top-left of the screen like seen
// on the issue comment menu.
Expand Down
11 changes: 6 additions & 5 deletions web_src/js/utils/color.ts
Original file line number Diff line number Diff line change
@@ -1,26 +1,27 @@
import tinycolor from 'tinycolor2';
import type {ColorInput} from 'tinycolor2';

// Returns relative luminance for a SRGB color - https://en.wikipedia.org/wiki/Relative_luminance
// Keep this in sync with modules/util/color.go
function getRelativeLuminance(color) {
function getRelativeLuminance(color: ColorInput) {
const {r, g, b} = tinycolor(color).toRgb();
return (0.2126729 * r + 0.7151522 * g + 0.072175 * b) / 255;
}

function useLightText(backgroundColor) {
function useLightText(backgroundColor: ColorInput) {
return getRelativeLuminance(backgroundColor) < 0.453;
}

// Given a background color, returns a black or white foreground color that the highest
// contrast ratio. In the future, the APCA contrast function, or CSS `contrast-color` will be better.
// https://github.com/color-js/color.js/blob/eb7b53f7a13bb716ec8b28c7a56f052cd599acd9/src/contrast/APCA.js#L42
export function contrastColor(backgroundColor) {
export function contrastColor(backgroundColor: ColorInput) {
return useLightText(backgroundColor) ? '#fff' : '#000';
}

function resolveColors(obj) {
function resolveColors(obj: Record<string, string>) {
const styles = window.getComputedStyle(document.documentElement);
const getColor = (name) => styles.getPropertyValue(name).trim();
const getColor = (name: string) => styles.getPropertyValue(name).trim();
return Object.fromEntries(Object.entries(obj).map(([key, value]) => [key, getColor(value)]));
}

Expand Down
Loading

0 comments on commit 32075d2

Please sign in to comment.