Skip to content

Commit

Permalink
PRfD UX UI Improvements (#8225)
Browse files Browse the repository at this point in the history
* Update src-dest UI in Pulls list

* Show closed/merged timestamp in the Pulls list

* Show merged commit link in merged PR details

* Cleanup

* Handle empty diffs

* Handle deleted branch errors

* Disable button when branch not found

* Update field name

* Fix PR comments
  • Loading branch information
itaigilo authored Sep 26, 2024
1 parent 7091bb3 commit 28bf39e
Show file tree
Hide file tree
Showing 5 changed files with 241 additions and 77 deletions.
13 changes: 12 additions & 1 deletion webui/src/lib/components/repository/compareBranches.jsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, {useState} from "react";
import React, {useContext, useEffect, useState} from "react";
import {refs as refsAPI} from "../../../lib/api";
import Alert from "react-bootstrap/Alert";
import {RefTypeBranch, RefTypeCommit} from "../../../constants";
Expand All @@ -8,10 +8,13 @@ import {AlertError, Loading} from "../controls";
import {ChangesTreeContainer, defaultGetMoreChanges} from "./changes";
import {URINavigator} from "./tree";
import CompareBranchesActionsBar from "./compareBranchesActionBar";
import {DiffActionType, DiffContext} from "../../hooks/diffContext";

const CompareBranches = (
{repo, reference, compareReference, showActionsBar, prefix = "", baseSelectURL}
) => {
const {dispatch} = useContext(DiffContext);

const [internalRefresh, setInternalRefresh] = useState(true);

const [afterUpdated, setAfterUpdated] = useState(""); // state of pagination of the item's children
Expand All @@ -34,6 +37,14 @@ const CompareBranches = (
const {results} = resultsState;
const apiResult = {results, loading, error, nextPage};

useEffect(() => {
// dispatch for dependent components
dispatch({
type: DiffActionType.setResults,
value: {results, loading, error, nextPage}
});
}, [results, loading, error, nextPage]);

const isEmptyDiff = (!loading && !error && !!results && results.length === 0);

const doRefresh = () => {
Expand Down
56 changes: 56 additions & 0 deletions webui/src/lib/hooks/diffContext.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import React, { createContext, useReducer } from "react";

type DiffContextType = {
results: object[] | null;
loading: boolean;
error: string | null;
nextPage: object | null;
};

enum DiffActionType {
setResults = 'setResults',
}

interface Action {
type: DiffActionType;
value: DiffContextType;
}

const initialDiffContext: DiffContextType = {
results: [],
loading: false,
error: null,
nextPage: null,
};

const diffContextReducer = (state: DiffContextType, action: Action) => {
switch (action.type) {
case DiffActionType.setResults:
return {...state, ...action.value};
default:
return state;
}
}

type ContextType = {
state: DiffContextType,
dispatch: React.Dispatch<Action>,
};

const DiffContext = createContext<ContextType>({
state: initialDiffContext,
dispatch: () => null
});

// @ts-expect-error - it doesn't like the "children" prop
const WithDiffContext: React.FC = ({children}) => {
const [state, dispatch] = useReducer(diffContextReducer, initialDiffContext);

return (
<DiffContext.Provider value={{state, dispatch}}>
{children}
</DiffContext.Provider>
)
}

export { WithDiffContext, DiffContext, DiffActionType };
29 changes: 21 additions & 8 deletions webui/src/pages/repositories/repository/pulls/createPull.jsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, {useEffect, useState} from "react";
import React, {useContext, useEffect, useState} from "react";
import {useOutletContext} from "react-router-dom";

import {ActionGroup, ActionsBar, AlertError, Loading} from "../../../../lib/components/controls";
Expand All @@ -11,6 +11,7 @@ import {RefTypeBranch} from "../../../../constants";
import Form from "react-bootstrap/Form";
import {pulls as pullsAPI} from "../../../../lib/api";
import CompareBranchesSelection from "../../../../lib/components/repository/compareBranchesSelection";
import {DiffContext, WithDiffContext} from "../../../../lib/hooks/diffContext";

const CreatePullForm = ({repo, reference, compare}) => {
const router = useRouter();
Expand All @@ -19,6 +20,9 @@ const CreatePullForm = ({repo, reference, compare}) => {
let [title, setTitle] = useState("");
let [description, setDescription] = useState("");

const {state: {results: diffResults, loading: diffLoading, error: diffError}} = useContext(DiffContext);
const isEmptyDiff = (!diffLoading && !diffError && !!diffResults && diffResults.length === 0);

const onTitleInput = ({target: {value}}) => setTitle(value);
const onDescriptionInput = ({target: {value}}) => setDescription(value);

Expand Down Expand Up @@ -67,12 +71,19 @@ const CreatePullForm = ({repo, reference, compare}) => {
/>
</Form.Group>
{error && <AlertError error={error} onDismiss={() => setError(null)}/>}
<Button variant="success"
disabled={!title || !description || loading}
onClick={submitForm}>
{loading && <><span className="spinner-border spinner-border-sm text-light" role="status"/> {""}</>}
Create Pull Request
</Button>
<div>
<Button variant="success"
disabled={!title || !description || loading || isEmptyDiff}
onClick={submitForm}>
{loading && <><span className="spinner-border spinner-border-sm text-light" role="status"/> {""}</>}
Create Pull Request
</Button>
{isEmptyDiff &&
<span className="alert alert-warning align-middle ms-4 pt-2 pb-2">
Pull requests must include changes.
</span>
}
</div>
</>;
};

Expand Down Expand Up @@ -111,7 +122,9 @@ const CreatePull = () => {
const RepositoryCreatePullPage = () => {
const [setActivePage] = useOutletContext();
useEffect(() => setActivePage("pulls"), [setActivePage]);
return <CreatePull/>;
return <WithDiffContext>
<CreatePull/>
</WithDiffContext>;
}

export default RepositoryCreatePullPage;
Loading

0 comments on commit 28bf39e

Please sign in to comment.