Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ change button style #2096

Closed
wants to merge 9 commits into from
4 changes: 4 additions & 0 deletions .github/workflows/ci-global.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ on:
- "main"

pull_request:
paths-ignore:
- "docs/**"
- "hack/**"
- "*.md"
branches:
- "main"

Expand Down
4 changes: 4 additions & 0 deletions .github/workflows/ci-image-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ name: CI (test image build for a PR with build related changes)

on:
pull_request:
paths-ignore:
- "docs/**"
- "hack/**"
- "*.md"
branches:
- "main"
- "release-*"
Expand Down
59 changes: 55 additions & 4 deletions .github/workflows/ci-repo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,26 +20,77 @@ concurrency:
cancel-in-progress: true

jobs:
unit-test-lookup-image:
unit-test-lookup:
runs-on: ubuntu-latest
outputs:
builder-image: ${{ steps.grepBuilder.outputs.builder }}
should-test: ${{ steps.check-changes.outputs.should-test }}

steps:
- uses: actions/checkout@v4

- name: Lookup builder image from the project's Dockerfile
id: grepBuilder
run: |
builder=$(grep 'as builder' Dockerfile | sed -e 's/^FROM \(.*\) as builder$/\1/')
echo "Builder image: \`$builder\`" >> "$GITHUB_STEP_SUMMARY"
echo "builder=$builder" >> "$GITHUB_OUTPUT"
- name: Did docs and hacks change?
id: docs-and-hacks
uses: tj-actions/changed-files@v44
with:
files: |
docs/**
hack/**
*.md
- name: Check if only docs and hacks changes have been made in a PR
id: check-changes
env:
IS_PR: ${{ !!github.event.pull_request }}
ONLY_DOCS: ${{ steps.docs-and-hacks.outputs.only_modified }}
run: |
SHOULD_TEST=$(
if [[ $IS_PR == true ]] && [[ $ONLY_DOCS == true ]]; then
echo "false"
else
echo "true"
fi
)
echo "is-pr=$IS_PR" >> "$GITHUB_OUTPUT"
echo "changes_only_docs=${ONLY_DOCS:-false}" >> "$GITHUB_OUTPUT"
echo "should-test=$SHOULD_TEST" >> "$GITHUB_OUTPUT"
- name: Summarize findings
env:
ONLY_DOCS: ${{ steps.docs-and-hacks.outputs.only_modified }}
MODIFIED_FILES: ${{ steps.docs-and-hacks.outputs.all_modified_files }}
run: |
cat >> "$GITHUB_STEP_SUMMARY" <<EOF
## Build container
- CI will run on container: \`${{ steps.grepBuilder.outputs.builder }}\`
## Findings
- The action is PR triggered? \`${{ steps.check-changes.outputs.is-pr }}\`
- Changes are only to docs and hacks? \`${{ steps.check-changes.outputs.changes_only_docs }}\`
- Should the unit test run? \`${{ steps.check-changes.outputs.should-test }}\`
EOF
if [[ $ONLY_DOCS == true ]] && [[ -n "$MODIFIED_FILES" ]]; then
echo "## Modified docs and hacks files that do not impact the build" >> "$GITHUB_STEP_SUMMARY"
for file in ${MODIFIED_FILES}; do
echo " - \`$file\`" >> "$GITHUB_STEP_SUMMARY"
done
fi
unit-test:
runs-on: ubuntu-latest
needs: unit-test-lookup-image
needs: unit-test-lookup
if: ${{ needs.unit-test-lookup.outputs.should-test == 'true' }}

# Use the same container as the Dockerfile's "FROM * as builder"
container: ${{ needs.unit-test-lookup-image.outputs.builder-image }}
container: ${{ needs.unit-test-lookup.outputs.builder-image }}

steps:
- uses: actions/checkout@v4
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/image-build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ jobs:
image-build:
uses: konveyor/release-tools/.github/workflows/build-push-images.yaml@main
with:
registry: "quay.io/konveyor"
image_name: "tackle2-ui"
registry: ${{ vars.IMAGE_BUILD_REGISTRY || 'quay.io/konveyor' }}
image_name: ${{ vars.IMAGE_BUILD_IMAGE_NAME || 'tackle2-ui' }}
containerfile: "./Dockerfile"

# keep the architectures in sync with `ci-image-build.yml`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
Select,
SelectList,
SelectOption,
ToolbarChip,
ToolbarFilter,
} from "@patternfly/react-core";
import { IFilterControlProps } from "./FilterControl";
Expand Down Expand Up @@ -56,8 +57,9 @@ export const SelectFilterControl = <TItem, TFilterCategoryKey extends string>({
setIsFilterDropdownOpen(false);
};

const onFilterClear = (chip: string) => {
const newValue = filterValue?.filter((val) => val !== chip);
const onFilterClear = (chip: string | ToolbarChip) => {
const chipValue = typeof chip === "string" ? chip : chip.key;
const newValue = filterValue?.filter((val) => val !== chipValue);
setFilterValue(newValue?.length ? newValue : null);
};

Expand Down Expand Up @@ -90,7 +92,7 @@ export const SelectFilterControl = <TItem, TFilterCategoryKey extends string>({
<ToolbarFilter
id={`filter-control-${category.categoryKey}`}
chips={chips}
deleteChip={(_, chip) => onFilterClear(chip as string)}
deleteChip={(_, chip) => onFilterClear(chip)}
categoryName={category.title}
showToolbarItem={showToolbarItem}
>
Expand Down
41 changes: 20 additions & 21 deletions client/src/app/components/InfiniteScroller/InfiniteScroller.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { ReactNode, useEffect, useRef } from "react";
import React, { ReactNode, useEffect, useState } from "react";
import { useTranslation } from "react-i18next";
import { useVisibilityTracker } from "./useVisibilityTracker";
import "./InfiniteScroller.css";
Expand All @@ -12,47 +12,46 @@ export interface InfiniteScrollerProps {
hasMore: boolean;
// number of items currently displayed/known
itemCount: number;
pageSize: number;
}

export const InfiniteScroller = ({
children,
fetchMore,
hasMore,
itemCount,
pageSize,
}: InfiniteScrollerProps) => {
const { t } = useTranslation();
// Track how many items were known at time of triggering the fetch.
// This allows to detect edge case when second(or more) fetchMore() is triggered before
// IntersectionObserver is able to detect out-of-view event.
// Initializing with zero ensures that the effect will be triggered immediately
// (parent is expected to display empty state until some items are available).
const itemCountRef = useRef(0);
const [readyForFetch, setReadyForFetch] = useState(false);
const { visible: isSentinelVisible, nodeRef: sentinelRef } =
useVisibilityTracker({
enable: hasMore,
});
useEffect(() => {
// enable or clear the flag depending on the sentinel visibility
setReadyForFetch(!!isSentinelVisible);
}, [isSentinelVisible]);

useEffect(
() => {
if (
isSentinelVisible &&
itemCountRef.current !== itemCount &&
fetchMore() // fetch may be blocked if background refresh is in progress (or other manual fetch)
) {
// fetchMore call was triggered (it may fail but will be subject to React Query retry policy)
itemCountRef.current = itemCount;
}
},
useEffect(() => {
if (readyForFetch) {
// clear the flag if fetch request is accepted
setReadyForFetch(!fetchMore());
}
// reference to fetchMore() changes based on query state and ensures that the effect is triggered in the right moment
// i.e. after fetch triggered by the previous fetchMore() call finished
[isSentinelVisible, fetchMore, itemCount]
);
}, [fetchMore, readyForFetch]);

return (
<div>
{children}
{hasMore && (
<div ref={sentinelRef} className="infinite-scroll-sentinel">
<div
ref={sentinelRef}
// re-create the node with every page change to force new Intersection observer
key={Math.ceil(itemCount / pageSize)}
className="infinite-scroll-sentinel"
>
{t("message.loadingTripleDot")}
</div>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { useEffect, useRef, useState, useCallback } from "react";

export function useVisibilityTracker({ enable }: { enable: boolean }) {
const nodeRef = useRef<HTMLDivElement>(null);
const [visible, setVisible] = useState(false);
const [visible, setVisible] = useState<boolean | undefined>(false);
const node = nodeRef.current;

// state is set from IntersectionObserver callbacks which may not align with React lifecycle
Expand All @@ -22,14 +22,19 @@ export function useVisibilityTracker({ enable }: { enable: boolean }) {
}, []);

useEffect(() => {
if (enable && !node) {
// use falsy value different than initial value - state change will trigger render()
// otherwise we need to wait for the next render() to read node ref
setVisibleSafe(undefined);
return undefined;
}

if (!enable || !node) {
return undefined;
}

// Observer with default options - the whole view port used.
// Note that if root element is used then it needs to be the ancestor of the target.
// In case of infinite scroller the target is always within the (scrollable!)parent
// even if the node is technically hidden from the user.
// https://developer.mozilla.org/en-US/docs/Web/API/Intersection_Observer_API#root
const observer = new IntersectionObserver(
(entries: IntersectionObserverEntry[]) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ interface TaskManagerDrawerProps {
export const TaskManagerDrawer: React.FC<TaskManagerDrawerProps> = forwardRef(
(_props, ref) => {
const { isExpanded, setIsExpanded, queuedCount } = useTaskManagerContext();
const { tasks, hasNextPage, fetchNextPage } = useTaskManagerData();
const { tasks, hasNextPage, fetchNextPage, pageSize } =
useTaskManagerData();

const [expandedItems, setExpandedItems] = useState<number[]>([]);
const [taskWithExpandedActions, setTaskWithExpandedAction] = useState<
Expand Down Expand Up @@ -106,6 +107,7 @@ export const TaskManagerDrawer: React.FC<TaskManagerDrawerProps> = forwardRef(
fetchMore={fetchNextPage}
hasMore={hasNextPage}
itemCount={tasks?.length ?? 0}
pageSize={pageSize}
>
<NotificationDrawerList>
{tasks.map((task) => (
Expand Down Expand Up @@ -282,6 +284,7 @@ const useTaskManagerData = () => {
[data]
);

// note that the callback will change when query fetching state changes
const fetchMore = useCallback(() => {
// forced fetch is not allowed when background fetch or other forced fetch is in progress
if (!isFetching && !isFetchingNextPage) {
Expand All @@ -297,6 +300,6 @@ const useTaskManagerData = () => {
isFetching,
hasNextPage,
fetchNextPage: fetchMore,
isReadyToFetch: !isFetching && !isFetchingNextPage,
pageSize: PAGE_SIZE,
};
};
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { parseMaybeNumericString } from "@app/utils/utils";
import { IFeaturePersistenceArgs } from "../types";
import { IFeaturePersistenceArgs, isPersistenceProvider } from "../types";
import { usePersistentState } from "@app/hooks/usePersistentState";

/**
Expand Down Expand Up @@ -76,7 +76,13 @@ export const useActiveItemState = <
persistTo,
key: "activeItem",
}
: { persistTo }),
: isPersistenceProvider(persistTo)
? {
persistTo: "provider",
serialize: persistTo.write,
deserialize: () => persistTo.read() as string | number | null,
}
: { persistTo: "state" }),
});
return { activeItemId, setActiveItemId };
};
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { usePersistentState } from "@app/hooks/usePersistentState";
import { objectKeys } from "@app/utils/utils";
import { IFeaturePersistenceArgs } from "../types";
import { IFeaturePersistenceArgs, isPersistenceProvider } from "../types";
import { DiscriminatedArgs } from "@app/utils/type-utils";

/**
Expand Down Expand Up @@ -93,7 +93,9 @@ export const useExpansionState = <
? {
persistTo,
keys: ["expandedCells"],
serialize: (expandedCellsObj) => {
serialize: (
expandedCellsObj: Partial<TExpandedCells<TColumnKey>>
) => {
if (!expandedCellsObj || objectKeys(expandedCellsObj).length === 0)
return { expandedCells: null };
return { expandedCells: JSON.stringify(expandedCellsObj) };
Expand All @@ -111,7 +113,13 @@ export const useExpansionState = <
persistTo,
key: "expandedCells",
}
: { persistTo }),
: isPersistenceProvider(persistTo)
? {
persistTo: "provider",
serialize: persistTo.write,
deserialize: () => persistTo.read() as TExpandedCells<TColumnKey>,
}
: { persistTo: "state" }),
});
return { expandedCells, setExpandedCells };
};
16 changes: 12 additions & 4 deletions client/src/app/hooks/table-controls/filtering/useFilterState.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { FilterCategory, IFilterValues } from "@app/components/FilterToolbar";
import { IFeaturePersistenceArgs } from "../types";
import { IFeaturePersistenceArgs, isPersistenceProvider } from "../types";
import { usePersistentState } from "@app/hooks/usePersistentState";
import { serializeFilterUrlParams } from "./helpers";
import { deserializeFilterUrlParams } from "./helpers";
Expand Down Expand Up @@ -90,7 +90,6 @@ export const useFilterState = <
"filters"
>({
isEnabled: !!isFilterEnabled,
defaultValue: initialFilterValues,
persistenceKeyPrefix,
// Note: For the discriminated union here to work without TypeScript getting confused
// (e.g. require the urlParams-specific options when persistTo === "urlParams"),
Expand All @@ -99,12 +98,21 @@ export const useFilterState = <
? {
persistTo,
keys: ["filters"],
defaultValue: initialFilterValues,
serialize: serializeFilterUrlParams,
deserialize: deserializeFilterUrlParams,
}
: persistTo === "localStorage" || persistTo === "sessionStorage"
? { persistTo, key: "filters" }
: { persistTo }),
? { persistTo, key: "filters", defaultValue: initialFilterValues }
: isPersistenceProvider(persistTo)
? {
persistTo: "provider",
serialize: persistTo.write,
deserialize: () =>
persistTo.read() as IFilterValues<TFilterCategoryKey>,
defaultValue: isFilterEnabled ? args?.initialFilterValues ?? {} : {},
}
: { persistTo: "state", defaultValue: initialFilterValues }),
});
return { filterValues, setFilterValues };
};
Loading
Loading