added status messages + code refactor (#3465)

* added status messages + code refactor

* Exclude `skipped` checks from running checks count

* Added types `ColorStyle` and `KindStyle`

- in order to share same color types across components and avoid duplication added  `ColorStyle` and `KindStyle` types
- Renamed color style `warn` in some components to `warning` for consistency

* typo and naming fixes

* Updated component style types

app/src/lib/components/Button.svelte

@@ -1,21 +1,10 @@
-<script lang="ts" context="module">
-	export type ButtonStyle =
-		| 'neutral'
-		| 'ghost'
-		| 'pop'
-		| 'success'
-		| 'error'
-		| 'warning'
-		| 'purple';
-	export type ButtonKind = 'soft' | 'solid';
-</script>
-
 <script lang="ts">
 	import Icon from '$lib/components/Icon.svelte';
 	import { pxToRem } from '$lib/utils/pxToRem';
 	import { tooltip } from '$lib/utils/tooltip';
 	import { onMount } from 'svelte';
 	import type iconsJson from '$lib/icons/icons.json';
+	import type { ComponentColor, ComponentStyleKind } from '$lib/vbranches/types';
 
 	// Interaction props
 	export let element: HTMLAnchorElement | HTMLButtonElement | HTMLElement | null = null;
@@ -34,8 +23,8 @@
 	export let align: 'flex-start' | 'center' | 'flex-end' | 'stretch' | 'baseline' | 'auto' = 'auto';
 	export let isDropdownChild = false;
 	// Style props
-	export let style: ButtonStyle = 'neutral';
-	export let kind: ButtonKind = 'soft';
+	export let style: ComponentColor = 'neutral';
+	export let kind: ComponentStyleKind = 'soft';
 	// Additional elements
 	export let icon: keyof typeof iconsJson | undefined = undefined;
 	export let help = '';

app/src/lib/components/CommitListHeader.svelte

@@ -24,7 +24,7 @@
 			Integrated
 		{:else if type == 'upstream'}
 			{commitCount} upstream {commitCount == 1 ? 'commit' : 'commits'}
-			<Icon name="warning" color="warn" />
+			<Icon name="warning" color="warning" />
 		{/if}
 	</div>
 	{#if isExpandable}

app/src/lib/components/CredentialCheck.svelte

@@ -48,7 +48,7 @@
 	{#if checks.length > 0}
 		<div transition:slide={{ duration: 250 }}>
 			<InfoMessage
-				style={errors > 0 ? 'warn' : loading ? 'neutral' : 'success'}
+				style={errors > 0 ? 'warning' : loading ? 'neutral' : 'success'}
 				filled
 				outlined={false}
 			>

app/src/lib/components/DropDownButton.svelte

@@ -1,11 +1,12 @@
 <script lang="ts">
 	import { clickOutside } from '$lib/clickOutside';
-	import Button, { type ButtonKind, type ButtonStyle } from '$lib/components/Button.svelte';
+	import Button from '$lib/components/Button.svelte';
 	import type iconsJson from '$lib/icons/icons.json';
+	import type { ComponentColor, ComponentStyleKind } from '$lib/vbranches/types';
 
 	export let icon: keyof typeof iconsJson | undefined = undefined;
-	export let style: ButtonStyle = 'neutral';
-	export let kind: ButtonKind = 'soft';
+	export let style: ComponentColor = 'neutral';
+	export let kind: ComponentStyleKind = 'soft';
 	export let disabled = false;
 	export let loading = false;
 	export let wide = false;

app/src/lib/components/FileDiff.svelte

@@ -59,7 +59,7 @@
 									delay: 500
 								}}
 							>
-								<Icon name="locked-small" color="warn" />
+								<Icon name="locked-small" color="warning" />
 							</div>
 						{/if}
 					</div>

app/src/lib/components/FileStatusIcons.svelte

@@ -20,7 +20,7 @@
 	<div class="file-status__icons">
 		{#if lockText}
 			<div class="locked" use:tooltip={{ text: lockText, delay: 500 }}>
-				<Icon name="locked-small" color="warn" />
+				<Icon name="locked-small" color="warning" />
 			</div>
 		{/if}
 		{#if file.conflicted}

app/src/lib/components/FileStatusTag.svelte

@@ -1,10 +1,11 @@
 <script lang="ts">
-	import Tag, { type TagStyle } from './Tag.svelte';
+	import Tag from './Tag.svelte';
 	import type { FileStatus } from '$lib/utils/fileStatus';
+	import type { ComponentColor } from '$lib/vbranches/types';
 
 	export let status: FileStatus;
 
-	function statusToColor(status: FileStatus): TagStyle {
+	function statusToColor(status: FileStatus): ComponentColor {
 		switch (status) {
 			case 'A':
 				return 'success';

app/src/lib/components/Icon.svelte

@@ -1,6 +1,7 @@
 <script lang="ts" context="module">
 	import { pxToRem } from '$lib/utils/pxToRem';
-	export type IconColor = 'success' | 'error' | 'pop' | 'warn' | 'neutral' | undefined;
+	import type { ComponentColor } from '$lib/vbranches/types';
+	export type IconColor = ComponentColor | undefined;
 </script>
 
 <script lang="ts">
@@ -14,13 +15,9 @@
 </script>
 
 <svg
-	class="icon-wrapper"
+	class="icon-wrapper {color}"
 	viewBox="0 0 16 16"
 	fill-rule="evenodd"
-	class:success={color == 'success'}
-	class:error={color == 'error'}
-	class:pop={color == 'pop'}
-	class:warn={color == 'warn'}
 	class:default={!color}
 	style:fill-opacity={opacity}
 	style:width={pxToRem(size)}
@@ -61,7 +58,7 @@
 	.pop {
 		color: var(--clr-scale-pop-50);
 	}
-	.warn {
+	.warning {
 		color: var(--clr-scale-warn-50);
 	}
 

app/src/lib/components/InfoMessage.svelte

@@ -1,9 +1,10 @@
 <script lang="ts" context="module">
-	export type MessageStyle = 'neutral' | 'error' | 'pop' | 'warn' | 'success';
+	import type { ComponentColor } from '$lib/vbranches/types';
+	export type MessageStyle = Exclude<ComponentColor, 'ghost' | 'purple'>;
 </script>
 
 <script lang="ts">
-	import Button, { type ButtonStyle } from './Button.svelte';
+	import Button from './Button.svelte';
 	import Icon, { type IconColor } from '$lib/components/Icon.svelte';
 	import { createEventDispatcher } from 'svelte';
 	import type iconsJson from '../icons/icons.json';
@@ -23,7 +24,7 @@
 	const iconMap: { [Key in MessageStyle]: keyof typeof iconsJson } = {
 		neutral: 'info',
 		pop: 'info',
-		warn: 'warning',
+		warning: 'warning',
 		error: 'error',
 		success: 'success'
 	};
@@ -31,27 +32,22 @@
 	const iconColorMap: { [Key in MessageStyle]: IconColor } = {
 		neutral: 'pop',
 		pop: 'pop',
-		warn: 'warn',
+		warning: 'warning',
 		error: 'error',
 		success: 'success'
 	};
 
-	const primaryButtonMap: { [Key in MessageStyle]: ButtonStyle } = {
+	const primaryButtonMap: { [Key in MessageStyle]: ComponentColor } = {
 		neutral: 'pop',
 		pop: 'pop',
-		warn: 'warning',
+		warning: 'warning',
 		error: 'error',
 		success: 'pop'
 	};
 </script>
 
 <div
-	class="info-message"
-	class:neutral={style == 'neutral'}
-	class:error={style == 'error'}
-	class:pop={style == 'pop'}
-	class:warn={style == 'warn'}
-	class:success={style == 'success'}
+	class="info-message {style}"
 	class:has-border={outlined}
 	class:has-background={filled}
 	class:shadow
@@ -137,7 +133,7 @@
 	.pop {
 		border: 0 solid var(--clr-scale-pop-50);
 	}
-	.warn {
+	.warning {
 		border: 0 solid var(--clr-scale-warn-60);
 	}
 	.success {
@@ -166,7 +162,7 @@
 			background-color: var(--clr-theme-pop-container);
 		}
 
-		&.warn {
+		&.warning {
 			background-color: var(--clr-theme-warn-container);
 		}
 

app/src/lib/components/PullRequestCard.svelte

@@ -1,7 +1,8 @@
 <script lang="ts">
 	import IconButton from './IconButton.svelte';
+	import InfoMessage from './InfoMessage.svelte';
 	import MergeButton from './MergeButton.svelte';
-	import Tag, { type TagStyle } from './Tag.svelte';
+	import Tag from './Tag.svelte';
 	import { Project } from '$lib/backend/projects';
 	import { BranchService } from '$lib/branches/service';
 	import ViewPrContextMenu from '$lib/components/ViewPrContextMenu.svelte';
@@ -14,11 +15,20 @@
 	import { Branch } from '$lib/vbranches/types';
 	import { onDestroy } from 'svelte';
 	import type { ChecksStatus, DetailedPullRequest } from '$lib/github/types';
+	import type { ComponentColor } from '$lib/vbranches/types';
+	import type { MessageStyle } from './InfoMessage.svelte';
 	import type iconsJson from '../icons/icons.json';
 	import type { Readable } from 'svelte/store';
 
 	export let isLaneCollapsed: boolean;
 
+	type StatusInfo = {
+		text: string;
+		icon: keyof typeof iconsJson | undefined;
+		style?: ComponentColor;
+		messageStyle?: MessageStyle;
+	};
+
 	const branch = getContextStore(Branch);
 	const branchService = getContext(BranchService);
 	const baseBranchService = getContext(BaseBranchService);
@@ -34,21 +44,17 @@
 	let mergeableState: string | undefined;
 	let checksStatus: ChecksStatus | null | undefined = undefined;
 	let lastDetailsFetch: Readable<string> | undefined;
-	let lastChecksFetch: Readable<string> | undefined;
 
 	$: pr$ = githubService.getPr$($branch.upstreamName);
 	$: if ($branch && $pr$) updateDetailsAndChecks();
 
-	$: checksIcon = getChecksIcon(checksStatus, isFetchingChecks);
-	$: checksColor = getChecksColor(checksStatus);
-	$: checksText = getChecksText(checksStatus);
-	$: statusIcon = getStatusIcon(detailedPr);
-	$: statusColor = getStatusColor(detailedPr);
-	$: statusLabel = getPrStatusLabel(detailedPr);
+	$: checksTagInfo = getChecksTagInfo(checksStatus, isFetchingChecks);
+	$: infoProps = getInfoMessageInfo(detailedPr, mergeableState, checksStatus, isFetchingChecks);
+	$: prStatusInfo = getPrStatusInfo(detailedPr);
 
 	async function updateDetailsAndChecks() {
-		if (!isFetchingChecks) fetchChecks();
-		if (!isFetchingDetails) updateDetailedPullRequest($pr$?.targetBranch, false);
+		if (!isFetchingDetails) await updateDetailedPullRequest($pr$?.targetBranch, true);
+		if (!isFetchingChecks) await fetchChecks();
 	}
 
 	async function updateDetailedPullRequest(targetBranch: string | undefined, skipCache: boolean) {
@@ -70,13 +76,12 @@
 	async function fetchChecks() {
 		checksError = undefined;
 		isFetchingChecks = true;
+
 		try {
 			checksStatus = await githubService.checks($pr$?.targetBranch);
-			lastChecksFetch = createTimeAgoStore(new Date(), true);
 		} catch (e: any) {
 			console.error(e);
 			checksError = e.message;
-			checksStatus = { error: 'could not load checks' };
 			if (!e.message.includes('No commit found')) {
 				toasts.error('Failed to fetch checks');
 			}
@@ -88,7 +93,6 @@
 	}
 
 	function scheduleNextUpdate() {
-		if (checksStatus?.error) return;
 		if (!checksStatus || checksStatus.completed) return;
 
 		const startedAt = checksStatus.startedAt;
@@ -115,67 +119,109 @@
 		setTimeout(() => updateDetailsAndChecks(), timeUntilUdate * 1000);
 	}
 
-	// TODO: Refactor away the code duplication in the following functions
-	function getChecksColor(status: ChecksStatus): TagStyle | undefined {
-		if (checksError || detailsError) return 'error';
-		if (!status) return 'neutral';
-		if (!status.hasChecks) return 'neutral';
-		if (status.error) return 'error';
-		if (status.completed) {
-			return status.success ? 'success' : 'error';
-		}
-		return 'warning';
+	function getChecksCount(status: ChecksStatus): string {
+		if (!status) return 'Running checks';
+
+		const skipped = status.skipped || 0;
+		const total = (status.totalCount || 0) - skipped;
+		const queued = total - (status.queued || 0);
+
+		return `Running checks ${queued}/${total}`;
 	}
 
-	function getChecksIcon(
-		status: ChecksStatus,
+	function getChecksTagInfo(
+		status: ChecksStatus | null | undefined,
 		fetching: boolean
-	): keyof typeof iconsJson | undefined {
-		if (checksError || detailsError) return 'warning-small';
-		if (status === null) return;
-		if (fetching || !status) return 'spinner';
-		if (!status.hasChecks) return;
-		if (status.error) return 'error-small';
-		if (status.completed) {
-			return status.success ? 'success-small' : 'error-small';
+	): StatusInfo {
+		if (checksError || detailsError) {
+			return { style: 'error', icon: 'warning-small', text: 'Failed to load' };
 		}
 
-		return 'spinner';
-	}
-
-	function getChecksText(status: ChecksStatus | undefined | null): string | undefined {
-		if (checksError || detailsError) return 'Failed to load';
-		if (!status) return 'Checks';
-		if (!status.hasChecks) return 'No checks';
-		if (status.error) return 'error';
-		if (status.completed) {
-			return status.success ? 'Checks passed' : 'Checks failed';
+		if (fetching || !status) {
+			return { style: 'neutral', icon: 'spinner', text: 'Checks' };
 		}
-		// Checking this second to last let's us keep the previous tag color unless
-		// checks are currently running.
-		if (isFetchingChecks) return 'Checks';
-		return 'Checks are running';
+
+		if (status.completed) {
+			const style = status.success ? 'success' : 'error';
+			const icon = status.success ? 'success-small' : 'error-small';
+			const text = status.success ? 'Checks passed' : 'Checks failed';
+			return { style, icon, text };
+		}
+
+		return {
+			style: 'warning',
+			icon: 'spinner',
+			text: getChecksCount(status)
+		};
 	}
 
-	function getPrStatusLabel(pr: DetailedPullRequest | undefined): string {
-		if (pr?.mergedAt) return 'Merged';
-		if (pr?.closedAt) return 'Closed';
-		if (pr?.draft) return 'Draft';
-		return 'Open';
+	function getPrStatusInfo(pr: DetailedPullRequest | undefined): StatusInfo {
+		if (!pr) {
+			return { text: 'Status', icon: 'spinner', style: 'neutral' };
+		}
+
+		if (pr?.mergedAt) {
+			return { text: 'Merged', icon: 'merged-pr-small', style: 'purple' };
+		}
+
+		if (pr?.closedAt) {
+			return { text: 'Closed', icon: 'closed-pr-small', style: 'error' };
+		}
+
+		if (pr?.draft) {
+			return { text: 'Draft', icon: 'draft-pr-small', style: 'neutral' };
+		}
+
+		return { text: 'Open', icon: 'pr-small', style: 'success' };
 	}
 
-	function getStatusIcon(pr: DetailedPullRequest | undefined): keyof typeof iconsJson | undefined {
-		if (pr?.mergedAt) return 'merged-pr-small';
-		if (pr?.closedAt) return 'closed-pr-small';
-		if (pr?.closedAt) return 'draft-pr-small';
-		return 'pr-small';
-	}
+	function getInfoMessageInfo(
+		pr: DetailedPullRequest | undefined,
+		mergeableState: string | undefined,
+		checksStatus: ChecksStatus | null | undefined,
+		isFetchingChecks: boolean
+	): StatusInfo | undefined {
+		if (mergeableState == 'blocked' && !checksStatus && !isFetchingChecks) {
+			return {
+				icon: 'error',
+				messageStyle: 'error',
+				text: 'Merge is blocked due to pending reviews or missing dependencies. Resolve the issues before merging.'
+			};
+		}
 
-	function getStatusColor(pr: DetailedPullRequest | undefined): TagStyle {
-		if (pr?.mergedAt) return 'purple';
-		if (pr?.closedAt) return 'error';
-		if (pr?.draft) return 'neutral';
-		return 'success';
+		if (checksStatus?.completed) {
+			if (pr?.draft) {
+				return {
+					icon: 'warning',
+					messageStyle: 'neutral',
+					text: 'This pull request is still a work in progress. Draft pull requests cannot be merged.'
+				};
+			}
+
+			if (mergeableState == 'unstable') {
+				return {
+					icon: 'warning',
+					messageStyle: 'warning',
+					text: 'Your PR is causing instability or errors in the build or tests. Review the checks and fix the issues before merging.'
+				};
+			}
+
+			if (mergeableState == 'dirty') {
+				return {
+					icon: 'warning',
+					messageStyle: 'warning',
+					text: 'Your PR has conflicts that must be resolved before merging.'
+				};
+			}
+
+			if (mergeableState == 'blocked' && !isFetchingChecks) {
+				return {
+					icon: 'error',
+					messageStyle: 'error',
+					text: 'Merge is blocked due to failing checks. Resolve the issues before merging.'
+				};
+			}
+		}
 	}
 
 	function updateContextMenu(copyablePrUrl: string) {
@@ -195,14 +241,14 @@
 	});
 </script>
 
-{#if $pr$?.htmlUrl}
+{#if $pr$}
 	{@const pr = $pr$}
 	<div class="card pr-card">
 		<div class="floating-button">
 			<IconButton
 				icon="update-small"
 				size="m"
-				loading={isFetchingDetails}
+				loading={isFetchingDetails || isFetchingChecks}
 				help={$lastDetailsFetch ? 'Updated ' + $lastDetailsFetch : ''}
 				on:click={async () => {
 					await updateDetailsAndChecks();
@@ -215,24 +261,21 @@
 		</div>
 		<div class="pr-tags">
 			<Tag
-				icon={statusIcon}
-				style={statusColor}
-				kind={statusLabel !== 'Open' ? 'solid' : 'soft'}
+				icon={prStatusInfo.icon}
+				style={prStatusInfo.style}
+				kind={prStatusInfo.text !== 'Open' && prStatusInfo.text !== 'Status' ? 'solid' : 'soft'}
 				verticalOrientation={isLaneCollapsed}
 			>
-				{statusLabel}
+				{prStatusInfo.text}
 			</Tag>
 			{#if !detailedPr?.closedAt && checksStatus !== null}
 				<Tag
-					icon={checksIcon}
-					style={checksColor}
-					kind={checksIcon == 'success-small' ? 'solid' : 'soft'}
-					clickable
+					icon={checksTagInfo.icon}
+					style={checksTagInfo.style}
+					kind={checksTagInfo.icon == 'success-small' ? 'solid' : 'soft'}
 					verticalOrientation={isLaneCollapsed}
-					on:mousedown={fetchChecks}
-					help={`Updated ${$lastChecksFetch}`}
 				>
-					{checksText}
+					{checksTagInfo.text}
 				</Tag>
 			{/if}
 			<Tag
@@ -242,7 +285,7 @@
 				clickable
 				shrinkable
 				verticalOrientation={isLaneCollapsed}
-				on:mousedown={(e) => {
+				on:click={(e) => {
 					const url = pr?.htmlUrl;
 					if (url) openExternalUrl(url);
 					e.preventDefault();
@@ -267,10 +310,19 @@
         -->
 		{#if pr}
 			<div class="pr-actions">
+				{#if infoProps}
+					<InfoMessage icon={infoProps.icon} filled outlined={false} style={infoProps.messageStyle}
+						>{infoProps.text}</InfoMessage
+					>
+				{/if}
+
 				<MergeButton
 					wide
 					projectId={project.id}
-					disabled={isFetchingChecks || pr?.draft || mergeableState != 'clean'}
+					disabled={isFetchingChecks ||
+						isFetchingDetails ||
+						pr?.draft ||
+						(mergeableState != 'clean' && mergeableState != 'unstable')}
 					loading={isMerging}
 					help="Merge pull request and refresh"
 					on:click={async (e) => {
@@ -317,6 +369,8 @@
 	.pr-actions {
 		margin-top: var(--size-14);
 		display: flex;
+		flex-direction: column;
+		gap: var(--size-8);
 	}
 
 	.floating-button {

app/src/lib/components/Tag.svelte

@@ -1,12 +1,8 @@
-<script lang="ts" context="module">
-	export type TagStyle = 'neutral' | 'ghost' | 'pop' | 'success' | 'error' | 'warning' | 'purple';
-	export type TagKind = 'soft' | 'solid';
-</script>
-
 <script lang="ts">
 	import Icon from '$lib/components/Icon.svelte';
 	import { tooltip } from '$lib/utils/tooltip';
 	import type iconsJson from '$lib/icons/icons.json';
+	import type { ComponentColor, ComponentStyleKind } from '$lib/vbranches/types';
 
 	// Interaction props
 	export let help = '';
@@ -19,8 +15,8 @@
 	export let icon: keyof typeof iconsJson | undefined = undefined;
 	export let reversedDirection = false;
 	// Style props
-	export let style: TagStyle = 'neutral';
-	export let kind: TagKind = 'soft';
+	export let style: ComponentColor = 'neutral';
+	export let kind: ComponentStyleKind = 'soft';
 </script>
 
 <div

app/src/lib/github/service.ts

@@ -383,7 +383,10 @@ export class GitHubService {
 			startedAt: firstStart,
 			hasChecks: !!totalCount,
 			success,
-			completed
+			completed,
+			queued,
+			totalCount,
+			skipped
 		};
 	}
 

app/src/lib/github/types.ts

@@ -61,7 +61,9 @@ export type ChecksStatus =
 			completed?: boolean;
 			success?: boolean;
 			hasChecks?: boolean;
-			error?: any;
+			queued?: number;
+			totalCount?: number;
+			skipped?: number;
 	  }
 	| null
 	| undefined;

app/src/lib/notifications/ToastController.svelte

@@ -1,6 +1,3 @@
-<script lang="ts" context="module">
-</script>
-
 <script lang="ts">
 	import InfoMessage from '$lib/components/InfoMessage.svelte';
 	import { dismissToast, toastStore } from '$lib/notifications/toasts';

app/src/lib/notifications/toasts.ts

@@ -1,12 +1,11 @@
 import { writable, type Writable } from 'svelte/store';
-
-export type ToastStyle = 'neutral' | 'error' | 'pop' | 'warn';
+import type { MessageStyle } from '$lib/components/InfoMessage.svelte';
 
 export interface Toast {
 	id?: string;
 	message: string;
 	title?: string;
-	style?: ToastStyle;
+	style?: MessageStyle;
 }
 
 export const toastStore: Writable<Toast[]> = writable([]);

app/src/lib/vbranches/types.ts

@@ -129,7 +129,15 @@ export class Branch {
 
 // Used for dependency injection
 export const BRANCH = Symbol('branch');
-
+export type ComponentStyleKind = 'solid' | 'soft';
+export type ComponentColor =
+	| 'neutral'
+	| 'ghost'
+	| 'pop'
+	| 'success'
+	| 'error'
+	| 'warning'
+	| 'purple';
 export type CommitStatus = 'local' | 'remote' | 'integrated' | 'upstream';
 
 export class Commit {