+
}
function Dot() {
@@ -42,7 +46,6 @@ function Dot() {
}
function Footer() {
- const {t} = useContext(ThemeContext);
const {status} = useContext(StatusContext);
let version;
try {
@@ -50,18 +53,19 @@ function Footer() {
} catch (e) {
// Not logging because this is not that important.
}
- return
-
+ return
+
WROLPi {version} WROLPi.org
Donate
-
+
}
function Root() {
return
+
@@ -87,6 +91,9 @@ const router = createBrowserRouter(createRoutesFromElements(}/>
}/>
}/>
+ {/* Component gallery: lets a user see a theme before committing to it, and lets us
+ review the design. Linked from Settings. */}
+ }/>
}>
}/>
@@ -118,8 +125,7 @@ export default function App() {
{/* Context and style to handle switching between mobile/computer. */}
- {/* Toasts can be on any page. */}
-
+ {/* Toasts (from components/ui) are mounted by ThemeProvider, which wraps every page. */}
diff --git a/app/src/DashboardPage.js b/app/src/DashboardPage.js
index 440b4d31..2ff83051 100644
--- a/app/src/DashboardPage.js
+++ b/app/src/DashboardPage.js
@@ -12,19 +12,21 @@ import {useHotkeys} from "react-hotkeys-hook";
import {Media, SettingsContext, StatusContext} from "./contexts/contexts";
import {DownloadMenu} from "./components/Download";
import {
+ ActionInput,
Button,
Divider,
- Form,
Header,
+ Icon,
+ Message,
Modal,
- Segment,
- Statistic
-} from "./components/Theme";
+ Panel,
+ Stack,
+ Statistic,
+ StatisticGroup,
+} from "./components/ui";
import {Link, useNavigate, useSearchParams} from "react-router";
import {BandwidthProgressCombined, CPUUsageProgress} from "./components/admin/Status";
import {ProgressPlaceholder} from "./components/Placeholder";
-import {GridColumn, GridRow, Icon, Input, Message} from "semantic-ui-react";
-import Grid from "semantic-ui-react/dist/commonjs/collections/Grid";
import {refreshFiles} from "./api";
import _ from "lodash";
import {TagsDashboard} from "./Tags";
@@ -37,6 +39,9 @@ import {fileMimetypeFilterOptions, FileCards, SearchFilterButton} from "./compon
import {useCalculators} from "./components/Calculators";
import {classifyAndEvaluate} from "./components/calculators/mathConfig";
+// A spinning refresh glyph, used as the `icon` for the "files are being refreshed" Message.
+const RefreshingIcon = (props) => ;
+
export function FlagsMessages() {
const {settings, fetchSettings} = React.useContext(SettingsContext);
const {status} = useContext(StatusContext);
@@ -53,47 +58,44 @@ export function FlagsMessages() {
// Do not tell the maintainer to refresh the files if the FileWorker is busy.
if (flags.file_worker_busy) {
// FileWorker is busy.
- refreshingMessage =
-
-
- Your files are being refreshed.
-
Click here to view the progress
-
+ refreshingMessage =
+
Click here to view the progress
;
} else if (!flags.refresh_complete) {
// `refresh_complete` flag is not set. Tell the maintainer to refresh the files.
- refreshRequiredMessage =
-
-
- Refresh required
- Click here to refresh all your files.
-
+ refreshRequiredMessage =
+ {
+ e.preventDefault();
+ refreshFiles();
+ }}>Click here to refresh all your files.
;
}
let dbDownMessage;
if (!flags.db_up) {
dbDownMessage =
- Database is down
- API is unable to connect to the database. Check the server logs.
+ Database is down
+
API is unable to connect to the database. Check the server logs.
}
let internetDownMessage;
if (!flags.have_internet && !settings.wrol_mode) {
internetDownMessage =
- No Internet
- WROLPi has no Internet. Downloads will not start.
+ No Internet
+
WROLPi has no Internet. Downloads will not start.
;
}
let mediaUnmountedMessage;
if (flags.media_mounted === false) {
- mediaUnmountedMessage =
- No drive mounted
- WROLPi has no media drive mounted for one or more storage locations.
- Downloads would write to the root filesystem and can fill it.
- {' '}Open the Controller to mount a drive.
+ mediaUnmountedMessage =
+ No drive mounted
+
+ WROLPi has no media drive mounted for one or more storage locations.
+ Downloads would write to the root filesystem and can fill it.
+ {' '}Open the Controller to mount a drive.
+
+ ;
let getterModal;
if (selectedGetter === 'downloads') {
- getterModal = handleSetGetter(null, null)}
@@ -281,7 +269,7 @@ export function Getters() {
;
} else if (selectedGetter === 'upload') {
- getterModal = handleSetGetter(null, null)}
@@ -304,7 +292,7 @@ export function Getters() {
}
-function DashboardStatus() {
+export function DashboardStatus() {
const {status} = useContext(StatusContext);
let percent = 0;
@@ -330,16 +318,25 @@ function DashboardStatus() {
bandwidths = null;
}
- return
-
+ return
+ {/*
+ * The panel is a link, but it should not read as one.
+ *
+ * `a` takes `color: var(--blue)` and an underline on hover from tokens.css, and a
+ * statistic's value sets no color of its own -- so the three load figures inherited
+ * the link color, and crossing the panel underlined them together. The same pair
+ * every card title uses says "this text belongs to what contains it": the pointer
+ * cursor still comes from the UA stylesheet, which is the affordance that matters.
+ */}
+
Status
-
+
-
+
Bandwidth
{bandwidths}
@@ -347,26 +344,26 @@ function DashboardStatus() {
-
-
+
+
-
+
- ;
+ ;
}
function DashboardRecentFiles() {
const {searchFiles, loading, fetchFiles} = useSearchRecentFiles();
- return
+ return
-
+
}
export function DashboardPage() {
@@ -467,7 +464,13 @@ export function DashboardPage() {
if (big) {
inputProps.size = 'big';
}
- return
+ /*
+ * No `marginBottom`. This row is a top-level block of the dashboard, so its own 2em was
+ * added to the page stack's gap -- 52.8px below the search box against 17.6px between
+ * every other pair of blocks, on the most-looked-at page in the app. One source of
+ * spacing, and it is the stack.
+ */
+ return
{getSearchResultsInput(inputProps)}
diff --git a/app/src/DashboardPage.test.js b/app/src/DashboardPage.test.js
index c86d2021..1d75414c 100644
--- a/app/src/DashboardPage.test.js
+++ b/app/src/DashboardPage.test.js
@@ -1,8 +1,7 @@
import React from 'react';
-import {render, screen} from '@testing-library/react';
-import {MemoryRouter} from 'react-router';
-import {Getters, FlagsMessages} from './DashboardPage';
-import {SettingsContext, StatusContext, ThemeContext} from './contexts/contexts';
+import {render, screen} from './test-utils';
+import {DashboardStatus, Getters, FlagsMessages} from './DashboardPage';
+import {SettingsContext, StatusContext} from './contexts/contexts';
// Replace DownloadMenu with a stub that records its props. The deep-link
// behavior under test is just "Getters reads URL params and forwards them
@@ -23,18 +22,14 @@ jest.mock('./hooks/customHooks', () => ({
}));
function renderAtUrl(url) {
+ // test-utils' render() wraps content in a BrowserRouter (not MemoryRouter),
+ // so point the jsdom location at the desired URL before rendering.
+ window.history.pushState({}, '', url);
const status = {flags: {refresh_complete: true}};
- const theme = {
- i: {}, s: {}, t: {}, theme: 'light', inverted: false, setInverted: () => {},
- };
return render(
-
-
-
-
-
-
-
+
+
+
);
}
@@ -80,13 +75,11 @@ describe('DashboardPage Getters deep-link', () => {
function renderFlagsMessages({flags = {}, settings = {}} = {}) {
const status = {flags: {refresh_complete: true, db_up: true, have_internet: true, ...flags}};
return render(
-
- {}}}>
-
-
-
-
-
+ {}}}>
+
+
+
+
);
}
@@ -108,3 +101,65 @@ describe('DashboardPage no-drive-mounted banner', () => {
expect(screen.queryByText(/No drive mounted/i)).toBeNull();
});
});
+
+function renderDashboardStatus() {
+ const status = {
+ cpu_stats: {percent: 12, cores: 4},
+ load_stats: {minute_1: 0.4, minute_5: 0.5, minute_15: 0.6},
+ downloads: {pending: 3, disabled: false},
+ nic_bandwidth_stats: {},
+ };
+ return render(
+
+
+
+ );
+}
+
+describe('the dashboard status panel links without looking like a link', () => {
+ /*
+ * The whole panel is wrapped in a to /admin/status, which makes it an anchor --
+ * and `a` takes `color: var(--blue)` with an underline on hover from tokens.css. A
+ * statistic's value sets no color of its own, so the load figures inherited that: three
+ * blue numbers on a panel where nothing else is blue, underlining as a group when the
+ * pointer crossed any part of the panel.
+ *
+ * `no-link-underscore card-link` is the pair already used wherever a link IS the
+ * element's own text rather than a link through it -- every card title and meta line.
+ * The pointer cursor is untouched: it comes from the UA stylesheet for `a[href]`, and
+ * the affordance is the point of wrapping the panel at all.
+ *
+ * Colors are asserted in ui-layout.cy.js; jsdom does not resolve the custom property
+ * these classes rely on.
+ */
+ it('dresses the status link as the panel\'s own text', () => {
+ const {container} = renderDashboardStatus();
+
+ const link = container.querySelector('a[href="/admin/status"]');
+ expect(link).toBeInTheDocument();
+ expect(link).toHaveClass('card-link');
+ expect(link).toHaveClass('no-link-underscore');
+ });
+
+ it('does the same for the downloads link beneath it', () => {
+ const {container} = renderDashboardStatus();
+
+ const link = container.querySelector('a[href="/admin"]');
+ expect(link).toBeInTheDocument();
+ expect(link).toHaveClass('card-link');
+ expect(link).toHaveClass('no-link-underscore');
+ });
+
+ it('leaves the load figures inside it, which is what went blue', () => {
+ // The premise for the two assertions above: this link really does wrap statistics
+ // whose value carries no color of its own, so it decides theirs.
+ const {container} = renderDashboardStatus();
+
+ const link = container.querySelector('a[href="/admin/status"]');
+ const values = [...link.querySelectorAll('.wrolpi-statistic-value')];
+
+ expect(values.length).toBeGreaterThanOrEqual(3);
+ expect(values.map(value => value.textContent)).toEqual(
+ expect.arrayContaining(['0.4', '0.5', '0.6']));
+ });
+});
diff --git a/app/src/Events.js b/app/src/Events.js
index be4b6795..3d21622b 100644
--- a/app/src/Events.js
+++ b/app/src/Events.js
@@ -1,7 +1,7 @@
import React, {useEffect} from 'react';
import {useRecurringTimeout} from "./hooks/customHooks";
import {ApiDownError, getEvents} from "./api";
-import {toast} from "react-semantic-toasts-2";
+import {toast} from "./components/ui";
const apiEventName = 'apiEvent';
diff --git a/app/src/Tags.cy.js b/app/src/Tags.cy.js
index 0644dfa2..a9919b9c 100644
--- a/app/src/Tags.cy.js
+++ b/app/src/Tags.cy.js
@@ -10,6 +10,8 @@ describe('Tags', () => {
it('Tags Dashboard displays Tags.', () => {
cy.wait('@getTags');
- cy.get('div.ui.label.large').contains('Automotive').should('be.visible');
+ // `.wrolpi-tag` is what a tag chip is now. `div.ui.label.large` named the old
+ // framework's markup, and has matched nothing since its stylesheet stopped loading.
+ cy.get('.wrolpi-tag').contains('Automotive').should('be.visible');
});
});
diff --git a/app/src/Tags.js b/app/src/Tags.js
index ce2f42f2..abcd0517 100644
--- a/app/src/Tags.js
+++ b/app/src/Tags.js
@@ -1,6 +1,5 @@
import React, {useEffect} from "react";
import {ApiDownError, deleteTag, getRecentTags, getTags, saveTag} from "./api";
-import {Dimmer, Divider, Grid, GridColumn, GridRow, Input, Label, TableCell, TableRow,} from "semantic-ui-react";
import {
APIButton,
contrastingColor,
@@ -10,17 +9,24 @@ import {
scrollToTopOfElement
} from "./components/Common";
import {
+ Box,
Button,
- Form,
+ Divider,
+ Group,
Header,
- Loader,
+ Icon,
+ IconButton,
+ Label,
+ Loading,
Modal,
- Segment
-} from "./components/Theme";
+ Panel,
+ Table,
+ TextInput,
+} from "./components/ui";
import _ from "lodash";
import {HexColorPicker} from "react-colorful";
import {useRecurringTimeout} from "./hooks/customHooks";
-import {Media, QueryContext, ThemeContext} from "./contexts/contexts";
+import {Media, QueryContext} from "./contexts/contexts";
import {Link, useNavigate} from "react-router";
import {TagPlaceholder} from "./components/Placeholder";
import {SortableTable} from "./components/SortableTable";
@@ -96,64 +102,96 @@ export function useTags() {
|| fuzzyMatch(lowerName, i.name.toLowerCase(), 2))
}
+ // A single tag chip. Tag colors are chosen by the user (stored per-tag), not by the
+ // theme, so they are rendered with the raw hex value rather than a token -- but the
+ // `wrolpi-label` className still gives them the theme's shape (and its night-mode
+ // outline treatment, which keys off the same `--label-color` variable).
const NameToTagLabel = ({name, to, ...props}) => {
const tag = findTagByName(name);
if (tag !== null && tag !== undefined) {
const tagColor = tag['color'] || DEFAULT_TAG_COLOR;
const textColor = contrastingColor(tagColor);
- const style = {...props['style'], backgroundColor: tagColor, color: textColor};
- return ;
+ ;
}
// No tags have been fetched.
- return ;
+ return ;
}
const TagsGroup = ({tagNames, onClick}) => {
if (!tagNames || tagNames.length === 0) {
return ;
}
- return
+ return
{tagNames.map(i => onClick(i)}/>)}
-
+
}
+ /*
+ * The link carries no margin of its own. It had `0.3em` either side, from when these were
+ * labels that had none -- but a tag already reserves a `margin-left` in ui.css for
+ * the point drawn outside its body, so that was 0.3em twice on top of the group's gap and
+ * the point's own room. Two tags sat 34.7px apart, of which the point needs 18.7px.
+ *
+ * Those are rendered px, measured in the browser at the default interface scale -- which is
+ * what a user sees, and why they are the numbers worth recording. The stylesheet states the
+ * same margin in design px, as `1.0625rem`; 17 x 1.1 is the 18.7 above.
+ */
const TagLabelLink = ({name, props}) => {
const to = getLocationStr({tag: name}, '/search');
- const style = {marginLeft: '0.3em', marginRight: '0.3em'};
try {
// We prefer to use Link to avoid reloading the page, check if React Router is available, so we can use it.
useNavigate();
- return
+ return
} catch {
// React Router is not available, use anchor.
- return
+ return
}
}
- const TagsLinkGroup = ({tagNames, ...props}) => {
+ /*
+ * Props here have two possible targets, and which one they take is part of the contract:
+ *
+ * `style` and `className` dress the ROW. Everything else reaches each CHIP, which is how
+ * `onClick` gets to a tag.
+ *
+ * `style` used to go down with the rest, so the dashboard's `marginTop` -- meant to space the
+ * row below its heading -- landed on all 34 tags and became 7.7px of extra space between
+ * every wrapped row of them. `className` is named alongside it because it is the same kind
+ * of thing and would fail the same way; a Mantine `Group` prop (`gap`, `wrap`, `justify`) or
+ * a `data-*` meant for the row still would, so add it here rather than at a call site.
+ */
+ const TagsLinkGroup = ({tagNames, style, className, ...props}) => {
if (!tagNames || tagNames.length === 0) {
return ;
}
- return
+ return
{tagNames.map(i => )}
-
+
}
const SingleTag = ({name, ...props}) => {
- return
+ return
}
useEffect(() => {
@@ -187,16 +225,15 @@ function EditTagRow({tag, onDelete, onEdit}) {
const {SingleTag} = React.useContext(TagsContext);
const {name, color, id, file_group_count, zim_entry_count, channel_count, domain_count} = tag;
- const deleteConfirm = <>
- onDelete(id, name)}
- />
- >;
- const editButton =
-
-
-
+
+ form.setValue('user_agent', e.target.value || null)}
/>
- Custom user agent string. Leave empty to use the system default.
-
+ Custom user agent string. Leave empty to use the system
+ default.
+
>;
}
function ArchiveSettingsPage() {
useTitle('Archive Settings');
- const {t} = React.useContext(ThemeContext);
const dockerized = useDockerized();
const [batchModalOpen, setBatchModalOpen] = useState(false);
const [domainsNeedingReorg, setDomainsNeedingReorg] = useState(0);
@@ -1001,47 +984,45 @@ function ArchiveSettingsPage() {
onClick={() => navigator.clipboard.writeText('url')}
/>;
- return
-
+ /*
+ * A fragment, not a `PageContainer`. `ArchiveRoute` already wraps its `` in one, and
+ * a second applies its `margin-top: 1em` and `padding: 1em` again -- so this page sat 1em
+ * lower and 1em further in on all four sides than /archives/domains beside it. As a fragment
+ * these panels become blocks of the route's own page stack, which spaces them.
+ */
+ return <>
+ Archive Downloader Config
-
-
+ File Organization
{fetchingReorgCount
@@ -1056,14 +1037,15 @@ function ArchiveSettingsPage() {
>;
}
export function CardGroupCentered(props) {
- return
-
-
- {props.children}
-
-
-
-
- {props.children}
-
-
+ // One responsive grid; it centres what it cannot fill, so the two viewports no
+ // longer need separate markup.
+ //
+ // No `marginTop`: this is a top-level block on every module page, and the page's stack
+ // spaces it. Carrying one added to that gap.
+ return
+ {props.children}
}
@@ -830,15 +757,11 @@ export function findPosterPath(file) {
}
}
-export function CardPoster({to, file}) {
- const {s} = useContext(ThemeContext);
- // Used to center posters in CardIcon.
- const style = {display: 'flex', justifyContent: 'center', ...s['style']};
+export function CardPoster({to, file, overlay}) {
const navigate = useNavigate();
- const cardTagIcon =
-
-
;
+ // Marks a tagged file, pinned to the poster's corner.
+ const cardTagIcon =
;
let imageLabel = !_.isEmpty(file.tags) ? cardTagIcon : null;
let posterPath = findPosterPath(file);
@@ -847,42 +770,47 @@ export function CardPoster({to, file}) {
// FileGroup has a poster (screenshot/thumbnail) file.
posterPath = `/media/${encodeMediaPath(posterPath)}`;
- const image = <>
- {/* Replicate but with maxHeight applied to image */}
+ /*
+ * Sized by `.wrolpi-card-poster`: as wide as the card allows, capped in height,
+ * natural ratio. The corner chrome goes INSIDE the link, beside the image:
+ *
+ * - the frame shrink-wraps the painted image, so a mark pinned to its corner
+ * tracks the poster rather than the letterbox space beside it. A height-capped
+ * poster is a good deal narrower than its band -- 155px inside a 233px card for
+ * a book cover, less for a vertical video;
+ * - and the mark stays part of the poster's hit target, rather than a dead spot
+ * on the corner of an otherwise clickable image.
+ */
+ const inner = <>
{imageLabel}
-
+
+ {overlay}
>;
- if (to) {
- // Link within this App.
- return
- {image}
-
- } else {
- // Preview the file.
- return
-
- {image}
-
-
- }
+ return
+ {to
+ // Link within this App.
+ ? {inner}
+ // Preview the file.
+ : {inner}}
+
} else {
// FileGroup has no poster.
if (!to || (to.startsWith('/media/') || to.startsWith('/download/'))) {
// "to" is a downloadable file outside the app, preview the file.
return
-
+
{imageLabel}
-
+
} else if (!posterPath && to) {
// Link to the full page in this App.
return
- navigate(to)}>
+
+ /*
+ * The icon goes in the heading's own `after` slot. It used to be a sibling ``
+ * beside a `;
if (popupContent) {
- return
+ return {body}
}
return body
}
@@ -1169,14 +1082,12 @@ export function DisableDownloadsToggle() {
}
const on = downloads && downloads['disabled'] === false && downloads['stopped'] === false;
- return ;
+ return ;
}
export function emptyToNull(obj) {
@@ -1355,43 +1266,73 @@ export function fileSuffixIconName(filename) {
return 'file';
}
-export function FileIcon({file, disabled = true, size = 'huge', ...props}) {
- // Default to a grey file icon.
+export function FileIcon({file, disabled = true, size = 48, ...props}) {
const {mimetype, path, primary_path} = file;
// `file` may be a file_group or a file.
const lowerPath = primary_path ? primary_path.toLocaleString() : path.toLowerCase();
- props['name'] = mimetypeIconName(mimetype, lowerPath);
- props['color'] = mimetypeColor(mimetype, lowerPath);
- return
+ const name = mimetypeIconName(mimetype, lowerPath);
+ // The mimetype picks a hue; the token picks what that hue is in this theme.
+ const color = mimetypeColor(mimetype, lowerPath);
+ return
}
-export function LoadStatistic({label, value, cores, ...props}) {
+/**
+ * The severity of a load average: a warning above half the cores, a problem above three
+ * quarters, and nothing to say below that or when the core count is unknown.
+ *
+ * Exported and pure so the branches can be tested as branches. Reading the color back off
+ * a rendered Statistic cannot do it -- jsdom rejects `color: var(--danger)` as an invalid
+ * declaration and drops it, so every value comes back as the empty string.
+ */
+export const loadRole = (value, cores) => {
+ if (!cores) return undefined;
const quarter = cores / 4;
- if (cores && value >= (quarter * 3)) {
- props['color'] = 'red';
- } else if (cores && value >= (quarter * 2)) {
- props['color'] = 'orange';
- }
+ if (value >= quarter * 3) return 'danger';
+ if (value >= quarter * 2) return 'warning';
+ return undefined;
+};
+
+export function LoadStatistic({label, value, cores, ...props}) {
+ const color = loadRole(value, cores);
return ;
}
export function DarkModeToggle() {
- const {savedTheme, cycleSavedTheme} = useContext(ThemeContext);
- let iconName = 'lightbulb outline';
- if (savedTheme === darkTheme) {
- iconName = 'moon';
- } else if (savedTheme === lightTheme) {
- iconName = 'sun';
- }
+ const {savedTheme, setTheme} = useContext(ThemeContext);
+ const active = themeChoices.find(i => i.value === savedTheme) || themeChoices[0];
- return
+ return
}
@@ -1400,28 +1341,36 @@ export function UnsupportedModal(header, message, icon) {
const onOpen = () => setOpen(true);
const onClose = () => setOpen(false);
- const modal =
-
-
- {header || 'Unsupported'}
-
-
-
- {message}
-
-
+ const modal =
+
+
+
+ {header || 'Unsupported'}
+
+
+ {message}
- Ok
+ Ok;
return {modal, doClose: onClose, doOpen: onOpen};
}
+/**
+ * The hotspot glyph in the nav bar: a wifi icon with a corner mark for the state.
+ * A button, not an anchor -- every one of these opens a dialog or toggles the
+ * hotspot, and `` was neither keyboard-honest nor navigable.
+ */
+function HotspotButton({label, corner, on, onClick}) {
+ const wifi = ;
+ return
+ corner
+ ? } label={label}>{wifi}
+ : wifi
+ }/>
+}
+
export function HotspotStatusIcon() {
const {on, inUse, setHotspot, dockerized, hotspotSsid} = useHotspot();
const {modal: unsupportedModal, doOpen: openUnsupportedModal} =
@@ -1451,55 +1400,39 @@ export function HotspotStatusIcon() {
return <>
setInUseOpen(false)}
- onClose={() => setInUseOpen(false)}
onConfirm={handleConfirmInUse}
- header='Wifi is in-use'
- content={content}
- confirmButton='Start Hotspot'
- />
- setInUseOpen(true)}>
-
-
-
-
-
+ title='Wifi is in-use'
+ confirmLabel='Start Hotspot'
+ >
+ {content}
+
+ setInUseOpen(true)}/>
>
} else if (dockerized === false && on === true) {
return <>
setStopHotspotOpen(false)}
- onClose={() => setStopHotspotOpen(false)}
onConfirm={handleConfirmStop}
- header='Stop the hotspot'
- content='You will be disconnected when using the hotspot. Are you sure?'
- confirmButton='Stop'
- />
- setStopHotspotOpen(true)}>
-
-
- {on === false && }
- {on === null && }
-
-
+ title='Stop the hotspot'
+ confirmLabel='Stop'
+ destructive
+ >
+ You will be disconnected when using the hotspot. Are you sure?
+
+ setStopHotspotOpen(true)}/>
>
} else if (dockerized === false && on === false) {
- return setHotspot(true)}>
-
-
-
-
-
+ return setHotspot(true)}/>
}
// Hotspot is not available, or, status has not yet been fetched.
return <>
-
-
-
-
-
-
+
{unsupportedModal}
>
}
@@ -1558,8 +1491,6 @@ export function DirectorySearch({onSelect, value, disabled, required, ...props})
const newDirectory = isDir ? {} : {
newDirectory: {
name: 'New Directory',
- // title must not be null; an unset form field passes a null value and
- // Semantic UI's Search crashes reading null.length.
results: [{title: directoryName || ''}],
}
};
@@ -1593,21 +1524,15 @@ export function DirectorySearch({onSelect, value, disabled, required, ...props})
loading,
]);
- const handleSearchChange = (e, data) => {
- if (e) {
- e.preventDefault();
- }
- setDirectoryName(data.value);
+ const handleSearchChange = (value) => {
+ setDirectoryName(value);
}
- const handleResultSelect = (e, data) => {
- if (e) {
- e.preventDefault();
- }
- setDirectoryName(data.result.title);
+ const handleResultSelect = (result) => {
+ setDirectoryName(result.title);
// title is the relative path.
if (onSelect) {
- onSelect(data.result.title);
+ onSelect(result.title);
}
}
@@ -1618,22 +1543,23 @@ export function DirectorySearch({onSelect, value, disabled, required, ...props})
}
}
- return
}
export const BackButton = ({...props}) => {
const navigate = useNavigate();
- return navigate(-1)} {...props}/>;
+ return navigate(-1)} {...props}>Back;
}
export const filterToMimetypes = (filter) => {
@@ -1669,26 +1595,45 @@ export const toLocaleString = (num, locale = 'en-US') => {
return num.toLocaleString(locale);
}
-function luma(color) {
- let rgb = (typeof color === 'string') ? hexToRGBArray(color) : color;
- return (0.2126 * rgb[0]) + (0.7152 * rgb[1]) + (0.0722 * rgb[2]); // SMPTE C, Rec. 709 weightings
+/*
+ * The WCAG primitives now live in themes/contrast, so the theme code can measure a color
+ * without importing this module -- Common.js pulls in half the component library, and
+ * `themes/` importing it is how an import cycle starts. Forwarded because a good many call
+ * sites and tests already take `contrastRatio` from here.
+ *
+ * It is forwarded by a function that delegates rather than by `export {contrastRatio}`, and
+ * that is load-bearing. A bare re-export compiles to a getter that dereferences the imported
+ * module, and six specs mock this module with `{...jest.requireActual('./Common')}` -- a
+ * spread invokes every getter, and those spreads run re-entrantly while Common.js is still
+ * evaluating (Common -> customHooks -> Common, both mocked that way). The getter fires
+ * before the import binding is assigned, throws `Cannot read properties of undefined`, and
+ * takes the whole suite with it. A function is read by the spread but not called, so it
+ * dereferences nothing until evaluation has finished.
+ *
+ * The hex parser accepting the `#rgb` shorthand matters and is not incidental: tag colors
+ * are not only chosen in the color picker, they live in a config file a user edits by hand
+ * -- configs are the source of truth in WROLPi. Rejecting `#fff` left the luminance at zero,
+ * so a near-white tag was treated as black and handed light text.
+ */
+export function contrastRatio(a, b) {
+ return measureContrast(a, b);
}
-function hexToRGBArray(color) {
- if (color.length === 7) {
- color = color.slice(1);
- }
- if (color.length !== 6) {
- console.error('Invalid hex color: ' + color);
- return;
- }
- let rgb = [];
- for (let i = 0; i <= 2; i++) rgb[i] = parseInt(color.substr(i * 2, 2), 16);
- return rgb;
-}
+export const TAG_TEXT_DARK = '#000000';
+export const TAG_TEXT_LIGHT = '#dddddd';
+/**
+ * Dark or light text, whichever a user's chosen tag color actually reads better against.
+ *
+ * Measures both instead of comparing a brightness figure to a threshold. The threshold
+ * version put light text on the palette blue (`#2185d0`) at 2.9:1 when dark text on the same
+ * fill gives 5.3:1 -- it had picked the worse of the only two options. Every mid-tone blue,
+ * teal and purple a user might choose sat in that band.
+ */
export function contrastingColor(color) {
- return (luma(color) >= 120) ? '#000000' : '#dddddd';
+ return contrastRatio(TAG_TEXT_DARK, color) >= contrastRatio(TAG_TEXT_LIGHT, color)
+ ? TAG_TEXT_DARK
+ : TAG_TEXT_LIGHT;
}
export const encodeMediaPath = (path) => {
@@ -1742,12 +1687,13 @@ export function SortButton({sorts = []}) {
let sortFields;
if (sorts && sorts.length) {
sortFields = sorts.map((i) => {
- return handleSortButton(i['value'])}>{i['text']}
+ return handleSortButton(i['value'])}>{i['text']}
})
}
return <>
- setOpen(false)}
>
@@ -1756,17 +1702,21 @@ export function SortButton({sorts = []}) {
{sortFields}
-
- toggleDesc()}/>
- setOpen(true)}/>
+
+ toggleDesc()}
+ />
+ setOpen(true)}>
+ {selectedSort['short'] || selectedSort['text']}
+
>
}
export function TagIcon() {
- return
-
-
+ return
}
export function normalizeEstimate(estimate) {
@@ -1778,7 +1728,19 @@ export function normalizeEstimate(estimate) {
export function useAPIButton(
color = 'violet',
- size = 'medium',
+ /*
+ * Undefined, not 'medium'. `size` was never put into `buttonArgs` below, so every
+ * APIButton in the app rendered at Mantine's default no matter what its call site asked
+ * for -- seventeen of them pass a size, from `xs` on the map's pin actions to `huge` on
+ * Settings, and all seventeen were ignored. The map's pin row is where it showed: three
+ * buttons meant to be `xs`, one of them 36px tall next to two 30px ones.
+ *
+ * Defaulting to 'medium' and passing that through would have fixed the bug by breaking
+ * everything else -- every APIButton that names no size would have grown from `sm` to
+ * `md`. Undefined lets Button's own default stand, so only the call sites that asked for
+ * something see a change.
+ */
+ size,
floated,
onClick,
disabled,
@@ -1797,8 +1759,6 @@ export function useAPIButton(
const [confirmOpen, setConfirmOpen] = React.useState(false);
const [loading, setLoading] = React.useState(false);
- const [animation, setAnimation] = React.useState('jiggle');
- const [animationVisible, setAnimationVisible] = React.useState(true);
const [showSuccess, setShowSuccess] = React.useState(false);
const [showFailure, setShowFailure] = React.useState(false);
@@ -1816,15 +1776,11 @@ export function useAPIButton(
const setSuccess = () => {
setShowSuccess(true);
- setAnimation('pulse');
- setAnimationVisible(!animationVisible);
setTimeout(reset, 2000);
}
const setFailure = () => {
setShowFailure(true);
- setAnimation('shake');
- setAnimationVisible(!animationVisible);
setTimeout(reset, 2000);
}
@@ -1867,50 +1823,52 @@ export function useAPIButton(
}
}
- // Create button with or without theme. Pass all props to the (except props.children).
+ // `color` was a bare color name; a role carries the meaning instead, and a
+ // caller that really wants a color can still pass one through `props`.
const buttonArgs = {
- color, onClick: localOnClick, disabled, loading, size, floated, type,
- ...props
+ onClick: localOnClick, disabled, loading, type, size,
+ ...props,
};
-
if (id) {
buttonArgs['id'] = id;
}
+ if (!props.color && !props.role) {
+ buttonArgs['role'] = 'primary';
+ }
+
+ // The result is shown as the icon rather than an animation: the outcome should be
+ // legible at a glance, and in night mode a moving element is the last thing wanted.
+ const outcomeIcon = showSuccess ? 'check' : showFailure ? 'close' : null;
+ // Roles are registered as Mantine colors, so these reach `color` the same way a hue
+ // would. This is the plainest success/failure in the app: the call worked, or it did not.
+ const outcomeColor = showSuccess ? 'success' : showFailure ? 'danger' : undefined;
+ if (outcomeColor) {
+ buttonArgs['color'] = outcomeColor;
+ }
let buttonContent = props.children || null;
if (icon) {
- // Send Icon as Button properties.
- buttonContent = null;
- buttonArgs['icon'] = showSuccess ? 'check' : showFailure ? 'close' : icon;
- } else if (showSuccess || showFailure) {
- // Show ✔ or ✖ overtop the contents after API call has completed.
- buttonContent = <>
-
- {/* Keep contents to avoid resizing button */}
-
}
diff --git a/app/src/components/Common.test.js b/app/src/components/Common.test.js
index 0a29d4c1..408b7804 100644
--- a/app/src/components/Common.test.js
+++ b/app/src/components/Common.test.js
@@ -1,7 +1,15 @@
import React from 'react';
-import {act, render, screen, waitFor} from '@testing-library/react';
+import fs from 'fs';
+import path from 'path';
+import {act, render, screen, waitFor} from '../test-utils';
import userEvent from '@testing-library/user-event';
-import {DirectorySearch} from './Common';
+import {
+ CardLink, contrastingColor, contrastRatio, DirectorySearch, ExternalCardLink, HelpHeader,
+ InfoHeader, loadRole,
+ PageContainer,
+ SearchResultsInput, TAG_TEXT_DARK, TAG_TEXT_LIGHT,
+} from './Common';
+import {healthRole} from './admin/ControllerPage';
// Mock debounce to make tests run faster
jest.mock('lodash/debounce', () => jest.fn(fn => {
@@ -102,7 +110,7 @@ describe('DirectorySearch', () => {
it('renders the dropdown without crashing when directoryName is null', async () => {
// An unset form field (e.g. the new Channel page) passes a null value. With
// isDir=false a "New Directory" result is built from directoryName; if that title is
- // null, Semantic UI's Search crashes reading null.length when the menu opens.
+ // null, the old Search crashed reading null.length when the menu opened.
setMockHookState({
directories: [],
channelDirectories: [],
@@ -123,13 +131,14 @@ describe('DirectorySearch', () => {
});
it('displays with required indicator', () => {
- const {container} = render(
+ render(
);
- // Semantic UI doesn't add required attribute to Search input,
- // but we verify the prop is passed
- expect(container.querySelector('.ui.search')).toBeInTheDocument();
+ // Unlike the old Search, the SearchBox forwards `required` straight
+ // onto the input.
+ const input = screen.getByPlaceholderText(/search directory names/i);
+ expect(input).toBeRequired();
});
});
@@ -153,11 +162,10 @@ describe('DirectorySearch', () => {
loading: true,
});
- render();
+ const {container} = render();
- const searchContainer = screen.getByPlaceholderText(/search directory names/i)
- .closest('.ui.search');
- expect(searchContainer).toHaveClass('loading');
+ // SearchBox renders a Loader alongside the input while `loading` is true.
+ expect(container.querySelector('.wrolpi-searchbox-loading')).toBeInTheDocument();
});
it('hides loading indicator when loading state is false', () => {
@@ -166,11 +174,9 @@ describe('DirectorySearch', () => {
loading: false,
});
- render();
+ const {container} = render();
- const searchContainer = screen.getByPlaceholderText(/search directory names/i)
- .closest('.ui.search');
- expect(searchContainer).not.toHaveClass('loading');
+ expect(container.querySelector('.wrolpi-searchbox-loading')).not.toBeInTheDocument();
});
it('displays categorized results', async () => {
@@ -265,16 +271,16 @@ describe('DirectorySearch', () => {
});
// Render with empty value prop (different from directoryName)
- const {container} = render();
+ render();
- // Find the Search component container and trigger blur on it
- const searchComponent = container.querySelector('.ui.search');
+ // The SearchBox wires `onBlur` straight to the input (there is no longer a
+ // wrapping Search component to blur instead).
+ const input = screen.getByPlaceholderText(/search directory names/i);
- // Blur the Search component - should trigger onBlur which calls onSelect
+ // Blur the input - should trigger onBlur which calls onSelect
await act(async () => {
- // Use fireEvent.blur which better simulates the Semantic UI Search blur behavior
- const {fireEvent} = require('@testing-library/react');
- fireEvent.blur(searchComponent);
+ const {fireEvent} = require('../test-utils');
+ fireEvent.blur(input);
});
// Should call onSelect with directoryName from hook state
@@ -319,8 +325,16 @@ describe('DirectorySearch', () => {
expect(screen.getByText('videos/nature')).toBeInTheDocument();
});
- // Click multiple results in succession
+ // Click multiple results in succession. Choosing a result closes the
+ // dropdown (as any combobox would) without moving focus off the input
+ // (the option is chosen on mousedown, with the default prevented), so
+ // pressing a key that reopens the list -- same as a real user -- is
+ // needed before the next selection.
await userEvent.click(screen.getByText('videos/nature'));
+ await userEvent.keyboard('{ArrowDown}');
+ await waitFor(() => {
+ expect(screen.getByText('videos/tech')).toBeInTheDocument();
+ });
await userEvent.click(screen.getByText('videos/tech'));
// Should call onSelect for each selection
@@ -436,3 +450,760 @@ describe('DirectorySearch', () => {
});
});
});
+
+describe('SearchResultsInput', () => {
+ const results = {
+ tags: {name: 'Tags', results: [{type: 'tag', title: 'Cooking'}]},
+ videos: {name: 'Videos', results: [
+ {title: 'Big Buck Bunny', description: 'Blender', location: '/videos/1'},
+ ]},
+ };
+
+ it('hands the selected suggestion to its caller as {result}', async () => {
+ /*
+ * Regression: the migration passed the bare result, but every caller destructures
+ * `{result}` (see Search.js), so `result.location` was undefined and clicking a
+ * search suggestion navigated nowhere.
+ */
+ const handleResultSelect = jest.fn();
+ render();
+
+ await userEvent.click(screen.getByRole('combobox'));
+ await userEvent.click(screen.getByText('Big Buck Bunny'));
+
+ expect(handleResultSelect).toHaveBeenCalledWith(
+ {result: expect.objectContaining({location: '/videos/1'})});
+ });
+
+ it('forwards resultRenderer so a caller keeps its own suggestion markup', async () => {
+ // Search.js renders a tag suggestion as a tag chip, not a line of text.
+ const resultRenderer = (result) => rendered:{result.title};
+ render();
+
+ await userEvent.click(screen.getByRole('combobox'));
+
+ expect(screen.getByText('rendered:Cooking')).toBeInTheDocument();
+ });
+});
+
+describe('contrastingColor', () => {
+ /*
+ * This decides the text color on every tag, against a fill the user picked, so it is the
+ * one place in the app where legibility is computed rather than designed.
+ */
+
+ it('picks whichever of the two options actually reads better', () => {
+ // The palette blue. The old threshold check chose light text here, at 2.9:1, when dark
+ // text on the same fill gives 5.3:1 -- the worse of the only two available answers.
+ // Every mid-tone blue, teal and purple a user might choose sat in that band.
+ const blue = '#2185d0';
+
+ const chosen = contrastingColor(blue);
+ const rejected = chosen === TAG_TEXT_DARK ? TAG_TEXT_LIGHT : TAG_TEXT_DARK;
+
+ expect(contrastRatio(chosen, blue)).toBeGreaterThan(contrastRatio(rejected, blue));
+ expect(chosen).toBe(TAG_TEXT_DARK);
+ });
+
+ it('never picks the worse option, for any color', () => {
+ // The property, rather than a list of examples: whatever it returns must be at least as
+ // legible as the alternative would have been.
+ const colors = [
+ '#000000', '#ffffff', '#f2f2f2', '#1b1c1d', '#2185d0', '#21ba45', '#db2828',
+ '#fbbd08', '#6435c9', '#a5673f', '#00b5ad', '#e03997', '#b5cc18', '#767676',
+ '#808080', '#7f7f7f',
+ ];
+
+ for (const color of colors) {
+ const chosen = contrastingColor(color);
+ const rejected = chosen === TAG_TEXT_DARK ? TAG_TEXT_LIGHT : TAG_TEXT_DARK;
+ expect(contrastRatio(chosen, color))
+ .toBeGreaterThanOrEqual(contrastRatio(rejected, color));
+ }
+ });
+
+ it('returns one of the two text colors and nothing else', () => {
+ // It is written into `--label-text`; anything undefined leaves the tag unstyled.
+ for (const color of ['#000000', '#ffffff', '#2185d0']) {
+ expect([TAG_TEXT_DARK, TAG_TEXT_LIGHT]).toContain(contrastingColor(color));
+ }
+ });
+
+ it('measures contrast on the WCAG scale', () => {
+ // Anchors the ratio itself, so "better" above is comparing real numbers.
+ expect(contrastRatio('#000000', '#ffffff')).toBeCloseTo(21, 1);
+ expect(contrastRatio('#ffffff', '#ffffff')).toBeCloseTo(1, 5);
+ });
+});
+
+describe('contrastingColor with shorthand hex', () => {
+ /*
+ * Tag colors reach the UI from the database, and a tags config is a file a user can edit
+ * by hand -- configs are the source of truth in WROLPi -- so a perfectly valid `#fff` can
+ * arrive here. `hexToRGBArray` used to reject anything but six digits, which left the
+ * luminance at zero: a near-white tag was treated as black and given light text.
+ */
+
+ it('reads a three-digit hex as the color it is', () => {
+ expect(contrastRatio('#000000', '#fff')).toBeCloseTo(21, 1);
+ expect(contrastRatio('#000000', '#ffffff')).toBeCloseTo(contrastRatio('#000000', '#fff'), 5);
+ });
+
+ it('picks dark text for a near-white shorthand fill', () => {
+ expect(contrastingColor('#fff')).toBe(TAG_TEXT_DARK);
+ });
+
+ it('picks light text for a near-black shorthand fill', () => {
+ expect(contrastingColor('#000')).toBe(TAG_TEXT_LIGHT);
+ });
+
+ it('agrees with the six-digit form of the same color', () => {
+ for (const [short, long] of [['#fff', '#ffffff'], ['#000', '#000000'],
+ ['#f00', '#ff0000'], ['#1b2', '#11bb22']]) {
+ expect(contrastingColor(short)).toBe(contrastingColor(long));
+ }
+ });
+});
+
+describe('loadRole', () => {
+ /*
+ * The branches, tested as branches. This used to be a source guard scraping the
+ * function body out of Common.js, because the obvious render test cannot work: jsdom
+ * REJECTS `color: var(--danger)` as an invalid declaration and drops it, so every inline
+ * color reads back as the empty string and the assertions passed for that reason alone.
+ * Pulling the mapping out as a pure function is the honest fix.
+ *
+ * Thresholds are halves and three quarters of the core count, so each is asserted either
+ * side of its boundary rather than at one comfortable value.
+ */
+ it('says nothing about a load below half the cores', () => {
+ expect(loadRole(1.9, 4)).toBeUndefined();
+ expect(loadRole(0, 4)).toBeUndefined();
+ });
+
+ it('warns from half the cores', () => {
+ expect(loadRole(2.0, 4)).toBe('warning');
+ expect(loadRole(2.9, 4)).toBe('warning');
+ });
+
+ it('calls it danger from three quarters', () => {
+ expect(loadRole(3.0, 4)).toBe('danger');
+ expect(loadRole(99, 4)).toBe('danger');
+ });
+
+ it('says nothing when the core count is unknown', () => {
+ // Both thresholds are guarded on `cores`. Without it every load would read as fine,
+ // which is the safe direction, but worth pinning so it stays deliberate.
+ expect(loadRole(12, undefined)).toBeUndefined();
+ expect(loadRole(12, 0)).toBeUndefined();
+ });
+
+ it('never returns a hue', () => {
+ // The regression this exists for: `orange` is byte-identical to `--text` in night,
+ // so a warning load rendered as an ordinary uncolored number.
+ [loadRole(1, 4), loadRole(2.5, 4), loadRole(4, 4)].filter(Boolean).forEach(role =>
+ expect(role).not.toMatch(/^(red|green|amber|yellow|orange|blue)$/));
+ });
+});
+
+describe('healthRole', () => {
+ it('maps every SMART assessment to a severity', () => {
+ expect(healthRole('PASS')).toBe('success');
+ expect(healthRole('WARN')).toBe('warning');
+ expect(healthRole('FAIL')).toBe('danger');
+ });
+
+ it('is not case sensitive, because the value comes from smartctl', () => {
+ expect(healthRole('pass')).toBe('success');
+ expect(healthRole('Fail')).toBe('danger');
+ });
+
+ it('treats a missing or unrecognised assessment as unknown, not as healthy', () => {
+ // Reporting an unreadable drive as PASS would be the dangerous direction.
+ [undefined, null, '', 'SOMETHING_ELSE'].forEach(value =>
+ expect(healthRole(value)).toBe('neutral'));
+ });
+});
+
+describe('a heading keeps its help icon on the same row as the text', () => {
+ /*
+ * `HelpHeader` and `InfoHeader` put their icon in a sibling `` beside a ``,
+ * held on one line by `.inline-header h1..h5 {display: inline-block}` in App.css. That
+ * worked while the heading was a bare `
`; the moment it gained a block wrapper the
+ * label's block child forced a break, and every help icon in the app dropped below its
+ * own heading -- visible on "Root CA Certificate" in Settings.
+ *
+ * They pass the icon through the heading's `after` slot now. Containment is the claim
+ * jsdom can judge; that the two actually share a line is asserted in ui-layout.cy.js,
+ * since jsdom has no layout at all.
+ */
+ it("renders the help control in the heading's row", () => {
+ const {container} = render(
+ );
+
+ expect(container.querySelector('.wrolpi-header-row'))
+ .toContainElement(screen.getByRole('button', {name: 'Help'}));
+ // And not inside the heading itself, which would put it in the accessible name.
+ expect(screen.getByRole('heading')).toHaveAccessibleName('Root CA Certificate');
+ });
+
+ it("renders the info popup in the heading's row", () => {
+ const {container} = render(
+ );
+
+ expect(container.querySelector('.wrolpi-header-after svg')).not.toBeNull();
+ });
+
+ it('renders no label at all when there is no field to name', () => {
+ /*
+ * Several call sites pass an InfoHeader as the `label` prop of InputForm/ToggleForm,
+ * which wraps it in a `` of its own. An unconditional label here nested one
+ * inside another -- invalid HTML, and it makes clicking the text ambiguous.
+ */
+ const {container} = render(
+ );
+
+ expect(container.querySelector('label')).toBeNull();
+ expect(screen.getByRole('heading', {name: /Video File Format/})).toBeInTheDocument();
+ });
+
+ it('keeps the label association, which is what `for_` is for', () => {
+ // Three call sites use it to tie the heading text to the field below it.
+ render();
+
+ expect(screen.getByText(/Video Resolutions/).closest('label'))
+ .toHaveAttribute('for', 'video_resolutions_input');
+ });
+
+ it('leaves no .inline-header behind', () => {
+ // The class and its stylesheet rules are gone; a stray one would be dead markup
+ // relying on CSS that no longer exists.
+ const {container} = render();
+
+ expect(container.querySelector('.inline-header')).toBeNull();
+ });
+});
+
+describe('card links', () => {
+ /*
+ * `.card-link` is what makes a card's title read as the card's text rather than as a
+ * link, so a title that misses it is the one thing on the card in the wrong color.
+ * Both helpers set the class and then spread the caller's props over it, so any call
+ * site passing a className of its own -- which is every archive title, via
+ * `card-title-ellipsis` -- silently replaced the class instead of adding to it.
+ */
+ it.each([
+ ['CardLink', Title],
+ ['ExternalCardLink',
+ Title],
+ ])('%s keeps its own classes when the call site adds one', (_name, element) => {
+ render(element);
+
+ const link = screen.getByText('Title');
+ expect(link).toHaveClass('card-link');
+ expect(link).toHaveClass('no-link-underscore');
+ expect(link).toHaveClass('card-title-ellipsis');
+ });
+});
+
+describe('PageContainer', () => {
+ /*
+ * The wrapper that decides how far apart a page's blocks sit. The spacing itself is a
+ * stylesheet rule and is measured in ui-layout.cy.js, where there is a layout engine; what
+ * this asserts is that the rule is REACHED -- the class has to be on both of the responsive
+ * branches, or the spacing silently applies on one viewport and not the other.
+ */
+ it('marks both of its viewport branches as a stack', () => {
+ const {container} = render(
Body
);
+
+ const stacks = container.querySelectorAll('.wrolpi-stack');
+ // Mobile and tablet-and-up, both rendered; `Media` hides one with CSS.
+ expect(stacks).toHaveLength(2);
+ stacks.forEach(stack => expect(stack.textContent).toBe('Body'));
+ });
+
+ it('is never rendered by a page whose route already provides page chrome', () => {
+ /*
+ * `PageContainer` carries `margin-top: 1em` and `padding: 1em`, so nesting one inside
+ * another applies both twice: the page is pushed 1em further down and inset 1em further
+ * in on all four sides than every other page in the app. `/archives/settings` did this
+ * -- `ArchiveRoute` wraps its `` in one, and `ArchiveSettingsPage` wrapped itself
+ * in another -- and read as "more margin than /archives/domains", which is exactly what
+ * it was.
+ *
+ * Nothing about a doubled margin throws, so this is a source scan. It finds each router
+ * whose enclosing `return` provides page chrome, takes the components it routes to, and
+ * checks the ones declared in the same file -- which is where a route and its pages
+ * normally live together.
+ *
+ * "Chrome" is `PageContainer` OR a `wrolpi-stack` wrapper, not just the former. Keying on
+ * `PageContainer` alone missed `MapRoute` and `ZimRoute`, which wrap their own content in a
+ * stack instead -- and three pages under them (`ManageMap`, `MapPins`, `ManageZim`) mounted
+ * a `PageContainer` each, so they sat a further 1em down and 1em in from the tab bar than
+ * the viewers beside them. The same defect as Archive, in a shape the first scan could not
+ * see.
+ *
+ * Known limit: a layout that routes through `` has its `` list in App.js,
+ * naming components from other files, so the page cannot be resolved from one file's text.
+ * That is `VideosTabLayout`, whose pages are clean today but are not covered here.
+ */
+ const src = SRC;
+ const files = () => sourceFiles();
+
+ const offenders = [];
+ for (const file of files(src)) {
+ const text = fs.readFileSync(file, 'utf8');
+ for (const block of chromedRouterBlocks(text)) {
+ for (const name of routedComponents(block)) {
+ if (bodyOf(text, name).includes('')) {
+ offenders.push(`${path.relative(src, file)}: ${name}`);
+ }
+ }
+ }
+ }
+
+ /*
+ * The premise: a scan that found no routers, or resolved no page components, would report
+ * no offenders and read exactly like a clean codebase.
+ */
+ const blocks = files(src).flatMap(file => chromedRouterBlocks(fs.readFileSync(file, 'utf8')));
+ expect(blocks.length).toBeGreaterThan(3);
+ expect(blocks.flatMap(routedComponents).length).toBeGreaterThan(8);
+
+ expect(offenders).toEqual([]);
+ });
+});
+
+describe('a row of buttons', () => {
+ /*
+ * Buttons used to bring `margin: 0 .25em .25em 0` with them, so a page could list buttons as
+ * bare siblings and they arrived both spaced and, when the row wrapped, separated. Nothing
+ * replaced that margin. The archive page's six actions shared edges, and two of them carried
+ * `marginTop: 0.5em` -- put there to keep a wrapped row from touching vertically, which instead
+ * made those two sit lower than the rest, because `vertical-align: middle` aligns the margin box.
+ *
+ * `.wrolpi-button-row` is a flex row with a gap, so it covers both axes at once and no button
+ * needs a margin of its own. ui-layout.cy.js measures that the row really spaces and aligns
+ * what it holds; these two scans are about the row being REACHED and left alone -- neither of
+ * which throws, and both of which is how the app got here.
+ */
+
+ /*
+ * `=>` is blanked before any tag scanning. An arrow function in a prop -- `onClick={() =>
+ * handleDelete(false)}`, which most of these buttons have -- puts a `>` inside an opening tag,
+ * and a scanner reading `>` as "tag ends here" splits one button into two bogus tags. Two
+ * characters for two, so every index still points where it did in the original text.
+ */
+ const scannable = (text) => text.replace(/=>/g, '=~');
+
+ const TAG = /<(\/?)([A-Za-z][\w.]*)([^>]*?)(\/?)>/g;
+ const tagsIn = (text) => [...scannable(text).matchAll(TAG)].map(match => ({
+ start: match.index,
+ end: match.index + match[0].length,
+ // The name was not captured until a test needed to accept `Group` and `Modal.Actions` as
+ // rows: `ROW_COMPONENTS.has(undefined)` is false for every tag, so the list of components
+ // that lay out their own children silently accepted none of them.
+ name: match[2],
+ attributes: match[3],
+ closing: match[1] === '/',
+ selfClosing: match[4] === '/',
+ }));
+
+ /**
+ * The text inside the element opened by `open`, found by walking forward to its MATCHING close.
+ *
+ * Written as "slice to the first `
`" first, which is true of every row in the app today
+ * and is a false negative waiting to happen: one nested div -- an error-trigger wrapper, a
+ * format-picker span, the Cancel/Save pair the collection form now groups -- truncates the body,
+ * and every button after it goes unscanned.
+ */
+ const bodyOfElement = (text, tags, open) => {
+ let depth = 0;
+ for (const tag of tags) {
+ if (tag.start <= open.start || tag.selfClosing) continue;
+ if (tag.closing) {
+ if (depth === 0) return text.slice(open.end, tag.start);
+ depth -= 1;
+ } else {
+ depth += 1;
+ }
+ }
+ return text.slice(open.end); // Unclosed; scan what is left rather than nothing.
+ };
+
+ /**
+ * The tag that encloses `index`: walk the tags before it backwards, letting a close push the
+ * depth up and an open bring it back down, and stop at the first open that is still unclosed.
+ */
+ const enclosingTag = (tags, index) => {
+ let depth = 0;
+ for (const tag of [...tags].filter(tag => tag.end <= index).reverse()) {
+ if (tag.selfClosing) continue;
+ if (tag.closing) depth += 1;
+ else if (depth === 0) return tag;
+ else depth -= 1;
+ }
+ return null;
+ };
+
+ /**
+ * Every `const = ...` declared at any indentation, as button consts are.
+ *
+ * Every one, not the first: Archive.js declares a `deleteButton` in the archive page and
+ * another in the domain edit page, and a resolver that stopped at the first found the clean one
+ * and reported the page with the margin on it as clean too.
+ */
+ const localBodiesOf = (text, name) => {
+ const pattern = new RegExp(`^[ \\t]*(?:const|let)\\s+${name}\\s*=`, 'gm');
+ return [...text.matchAll(pattern)].map(found => {
+ const after = text.slice(found.index + found[0].length);
+ const next = after.search(/\n[ \t]*(?:const|let|function|return|\})[\s(]/);
+ return next === -1 ? after : after.slice(0, next);
+ });
+ };
+
+ /** Every `const actionButtons = <>...>` fragment in `text`: a button row under another name. */
+ const actionFragments = (text) => {
+ const bodies = [];
+ const pattern = /^[ \t]*const\s+actionButtons\s*=\s*<>/gm;
+ for (const found of text.matchAll(pattern)) {
+ const from = found.index + found[0].length;
+ const end = text.indexOf('>', from);
+ if (end !== -1) bodies.push(text.slice(from, end));
+ }
+ return bodies;
+ };
+
+ const VERTICAL_MARGIN = /margin(?:Top|Bottom)\s*:|margin\s*:/;
+
+ /*
+ * The classes that lay buttons out in a row. A row is a row whatever it is called -- the file
+ * preview's toolbar had one before this, at a tighter gap because its buttons are icons -- so
+ * the check below is "is this one of them", and the test after it is what keeps the list honest
+ * by reading each one out of the stylesheet and confirming it really is a flex row with a gap.
+ */
+ const ROW_CLASSES = [
+ 'wrolpi-button-row', 'wrolpi-modal-actions', 'wrolpi-confirm-actions', 'preview-toolbar-group',
+ // Two that predate `.wrolpi-button-row` and are already correct: the file browser's sticky
+ // footer, and its filter row. Both are checked against the stylesheet below like the rest.
+ 'sticky-footer', 'file-browser-filter-container',
+ ];
+
+ /*
+ * Components that lay their own children out in a row. A `Group` or a `ButtonGroup` is a row by
+ * construction, so a caller putting buttons in one has already answered the question.
+ */
+ const ROW_COMPONENTS = new Set(['Group', 'ButtonGroup', 'Row', 'Stack', 'Modal.Actions']);
+
+ /*
+ * An inline flex or grid with a gap of its own -- an anonymous row. Several call sites write one
+ * rather than reach for a class, and they are not wrong; a scan that only knew about class names
+ * would report every one of them.
+ */
+ const INLINE_ROW = /display:\s*['"]?(?:flex|grid)/;
+ const isInlineRow = (attributes, text) => {
+ if (INLINE_ROW.test(attributes) && attributes.includes('gap')) return true;
+ /*
+ * `style={someStyle}` -- the style is a const somewhere above, so the attribute itself says
+ * nothing. Resolving it matters: the seasons row on the dates selector is a four-column
+ * grid with a gap, declared exactly that way, and it would otherwise be reported forever.
+ */
+ const named = /style=\{(\w+)\}/.exec(attributes);
+ if (!named) return false;
+ const declared = new RegExp(`\\b${named[1]}\\s*=\\s*(\\{[^;]*?\\});`).exec(text);
+ return !!declared && INLINE_ROW.test(declared[1]) && declared[1].includes('gap');
+ };
+
+ const isRow = (tag, text = '') => !!tag && (
+ ROW_COMPONENTS.has(tag.name)
+ || ROW_CLASSES.some(name => tag.attributes.includes(name))
+ || isInlineRow(tag.attributes, text));
+
+ it('lays every row out as a flex row with a gap', () => {
+ // Otherwise the list above is just a list of names the scans below happen to accept.
+ const stylesheets = ['components/ui/ui.css', 'App.css']
+ .map(name => fs.readFileSync(path.join(SRC, name), 'utf8')).join('\n');
+
+ ROW_CLASSES.forEach(name => {
+ const found = new RegExp(`\\.${name}\\s*\\{([^}]*)\\}`).exec(stylesheets);
+ // Named in the failure rather than passed to `expect`, which takes no message.
+ expect(found ? name : `${name} is not declared`).toBe(name);
+ expect(found[1]).toMatch(/display:\s*flex/);
+ expect(found[1]).toMatch(/gap:/);
+ });
+ });
+
+ it('never stamps a vertical margin on a button the row already spaces', () => {
+ /*
+ * The margin and the gap add up. Nine buttons across the three collection edit pages had
+ * `marginTop: 1em` each and sat 1em below the Save button beside them; the archive page's
+ * Update and Generate Screenshot had 0.5em and sat half that below their own neighbours.
+ */
+ const offenders = [];
+ let rows = 0;
+ let resolved = 0;
+
+ for (const file of sourceFiles()) {
+ const text = fs.readFileSync(file, 'utf8');
+ const where = path.relative(SRC, file);
+
+ const blocks = actionFragments(text);
+ const tags = tagsIn(text);
+ for (const tag of tags) {
+ if (!tag.closing && tag.attributes.includes('wrolpi-button-row')) {
+ blocks.push(bodyOfElement(text, tags, tag));
+ }
+ }
+ rows += blocks.length;
+
+ for (const block of blocks) {
+ // A button declared inline in the row, style and all.
+ if (VERTICAL_MARGIN.test(block)) offenders.push(`${where}: inline in the row`);
+ // ...or interpolated by name, which is how most of them are written.
+ for (const name of new Set([...block.matchAll(/\{(\w*[Bb]utton\w*)\}/g)]
+ .map(match => match[1]))) {
+ for (const body of localBodiesOf(text, name)) {
+ resolved += 1;
+ if (VERTICAL_MARGIN.test(body)) offenders.push(`${where}: ${name}`);
+ }
+ }
+ }
+ }
+
+ // A scan that found no rows, or resolved none of their buttons, reports a clean app.
+ expect(rows).toBeGreaterThan(3);
+ expect(resolved).toBeGreaterThan(8);
+
+ expect(offenders).toEqual([]);
+ });
+
+ it('puts every row of buttons in a row', () => {
+ /*
+ * The other half: a page can have margin-free buttons and still list them as bare siblings,
+ * which is the touching this fixes. Every `{actionButtons}` is rendered somewhere, and the
+ * element it is rendered into is what has to carry the gap -- on the doc page that was a
+ * plain `
`, spacing the row from what was above it while
+ * leaving the buttons inside it edge to edge.
+ */
+ const offenders = [];
+ let sites = 0;
+
+ for (const file of sourceFiles()) {
+ const text = fs.readFileSync(file, 'utf8');
+ if (!text.includes('{actionButtons}')) continue;
+ const tags = tagsIn(text);
+
+ for (const found of text.matchAll(/\{actionButtons\}/g)) {
+ // `actionButtons={actionButtons}` is the prop being handed down, not a render.
+ if (text.slice(found.index - 14, found.index) === 'actionButtons=') continue;
+ sites += 1;
+ const parent = enclosingTag(tags, found.index);
+ if (!isRow(parent)) {
+ offenders.push(`${path.relative(SRC, file)}: ${
+ parent ? parent.attributes.trim().slice(0, 40) : 'no enclosing tag'}`);
+ }
+ }
+ }
+
+ /*
+ * Three: the shared collection edit form, which three pages hand a fragment to, and the doc
+ * page's own two responsive branches. A scan that counted the `actionButtons={...}` props
+ * instead would reach eight and never look at a render site at all.
+ */
+ expect(sites).toBe(3);
+
+ expect(offenders).toEqual([]);
+ });
+
+ it('never lists interpolated buttons as bare siblings', () => {
+ /*
+ * The touching itself, and the reported defect: the archive page interpolated six buttons
+ * one after another straight into its Panel, so every pair shared an edge.
+ *
+ * A run of adjacent button interpolations is the shape to look for. Runs inside an
+ * `actionButtons` fragment are blanked first -- a fragment is spread into its parent's row
+ * and has no business being one itself, which the previous test is what checks. Blanked
+ * rather than skipped so every index still points where it did.
+ *
+ * The sibling test below covers the other way a row is written; this one only sees `{foo}`.
+ */
+ const offenders = [];
+ let runs = 0;
+
+ for (const file of sourceFiles()) {
+ let text = fs.readFileSync(file, 'utf8');
+ for (const fragment of actionFragments(text)) {
+ text = text.replace(fragment, ' '.repeat(fragment.length));
+ }
+ const tags = tagsIn(text);
+
+ for (const found of text.matchAll(/\{\w*[Bb]utton\w*\}(?:\s*\{\w*[Bb]utton\w*\})+/g)) {
+ runs += 1;
+ const parent = enclosingTag(tags, found.index);
+ if (!isRow(parent)) {
+ offenders.push(`${path.relative(SRC, file)}: ${found[0].replace(/\s+/g, '')}`);
+ }
+ }
+ }
+
+ /*
+ * Four: the archive page's actions, the file preview's toolbar (which had a row of its own
+ * already), the downloads table's edit/restart pair, and the doc page's format picker
+ * beside its actions. Asserted exactly, because a run that stopped being recognised would
+ * leave this reporting a clean app.
+ */
+ expect(runs).toBe(4);
+
+ expect(offenders).toEqual([]);
+ });
+
+ it('never lists written-out buttons as bare siblings either', () => {
+ /*
+ * The same claim for the other way a row is written. The interpolation scan above sees
+ * `{fooButton}` and nothing else, and most of the app writes its buttons out in place --
+ * which is how the video page's Download/Delete/Refresh/AddToPlaylist strip, the direct
+ * analogue of the archive row this PR started with, went on touching after that scan
+ * reported the app clean. Twelve more rows were in the same state, in table cells, on the
+ * controller page, and under the one-time-pad and ration calculators.
+ *
+ * A "row" is deliberately broad here: the class, a Group or ButtonGroup, or an inline
+ * flex/grid with a gap, including one whose style is a const declared above. The point is
+ * whether the buttons are spaced by something, not whether they use this PR's class.
+ */
+ const offenders = [];
+ let runs = 0;
+
+ for (const file of sourceFiles()) {
+ let text = fs.readFileSync(file, 'utf8');
+ // A fragment is spread into its parent's row; the earlier test checks that parent.
+ for (const fragment of actionFragments(text)) {
+ text = text.replace(fragment, ' '.repeat(fragment.length));
+ }
+ const tags = tagsIn(text);
+
+ // A button element ending, then whitespace, then another button beginning.
+ const runPattern = /(?:<\/\w*Button>|<\w*Button\b[^>]*?\/>)\s*<\w*Button\b/g;
+ for (const match of scannable(text).matchAll(runPattern)) {
+ // Anchor on the SECOND button's `<`: at the first one's closing tag the pair is
+ // still open, and the walk would report the first button as its own parent.
+ const at = match.index + match[0].lastIndexOf('<');
+ runs += 1;
+ if (!isRow(enclosingTag(tags, at), text)) {
+ offenders.push(`${path.relative(SRC, file)}:${
+ text.slice(0, at).split('\n').length}`);
+ }
+ }
+ }
+
+ /*
+ * Around eighty, which is why this is a scan and not a list. Asserted loosely at both ends
+ * -- tight enough that a pattern matching nothing shows up, loose enough that adding a
+ * button to an existing row is not a test failure.
+ */
+ expect(runs).toBeGreaterThan(60);
+ expect(runs).toBeLessThan(200);
+
+ expect(offenders).toEqual([]);
+ });
+
+ it('puts every Back button in a row, so the page stack cannot stretch it', () => {
+ /*
+ * A page's blocks are stacked by `.wrolpi-stack`, which is a flex column at
+ * `align-items: stretch` -- deliberately, because a panel was full-width as a block
+ * element and has to stay that way. A BARE button as a direct child of that stack is
+ * stretched too, and every page's Back button was one: full-page-width, and on the domain,
+ * channel and playlist edit pages the button beside it was pushed onto its own line where
+ * the two had been inline before.
+ *
+ * A row fixes it without touching the stack: the row is the stretched child, and it lays
+ * its buttons out at their own width. Nine sites, so this scan rather than nine
+ * assertions -- and a scan because a stretched button is still a working button.
+ */
+ const offenders = [];
+ let found = 0;
+
+ for (const file of sourceFiles()) {
+ const text = fs.readFileSync(file, 'utf8');
+ const tags = tagsIn(text);
+
+ for (const match of text.matchAll(/ fs.readdirSync(dir, {withFileTypes: true}).flatMap(entry => {
+ const full = path.join(dir, entry.name);
+ if (entry.isDirectory()) return sourceFiles(full);
+ if (!/\.jsx?$/.test(entry.name)) return [];
+ if (/\.(test|cy)\.jsx?$/.test(entry.name)) return [];
+ return [full];
+});
+
+/**
+ * Every `...` in `text` whose enclosing `return` provides page chrome -- a
+ * `PageContainer` or a `wrolpi-stack` wrapper. Those are the routers that have already spaced and
+ * inset the page, so a page under one must not do it again.
+ *
+ * All of them, and located from the router rather than from the chrome: keying on the first
+ * `` in a file missed this entirely in Archive.js, where the first one belongs to
+ * the offending page and holds no router at all.
+ */
+const chromedRouterBlocks = (text) => {
+ const blocks = [];
+ let from = 0;
+ for (;;) {
+ const start = text.indexOf('', from);
+ if (start === -1) return blocks;
+ const end = text.indexOf('', start);
+ if (end === -1) return blocks;
+ from = end + 1;
+
+ const returned = text.lastIndexOf('return', start);
+ if (returned === -1) continue;
+ const chrome = text.slice(returned, start);
+ if (chrome.includes('') || chrome.includes('wrolpi-stack')) {
+ blocks.push(text.slice(start, end));
+ }
+ }
+};
+
+/**
+ * The page components a router block routes to. Any capitalised tag that is not routing furniture,
+ * so `element={}` yields `Page` as well as the plain
+ * `element={}` form.
+ */
+const FURNITURE = new Set(['Route', 'Routes', 'ErrorBoundary', 'Suspense', 'Navigate', 'Outlet']);
+const routedComponents = (block) => [...new Set(
+ [...block.matchAll(/<([A-Z]\w*)/g)].map(m => m[1]).filter(name => !FURNITURE.has(name)))];
+
+/**
+ * The source of the component named `name`, bounded by the next top-level declaration. `class` is
+ * in both patterns because `ManageZim` is one, and a scan that only knew about functions would
+ * have reported it clean.
+ */
+const bodyOf = (text, name) => {
+ const declaration = new RegExp(`^(?:export\\s+)?(?:function|const|class)\\s+${name}\\b`, 'm');
+ const found = declaration.exec(text);
+ if (!found) return ''; // Declared elsewhere; out of this scan's reach.
+ const after = text.slice(found.index + 1);
+ const next = after.search(/^(?:export\s+)?(?:function|const|class)\s/m);
+ return next === -1 ? after : after.slice(0, next);
+};
diff --git a/app/src/components/DatesSelector.js b/app/src/components/DatesSelector.js
index 14974200..6aa8bd4c 100644
--- a/app/src/components/DatesSelector.js
+++ b/app/src/components/DatesSelector.js
@@ -1,9 +1,6 @@
import React from "react";
-import {Button, Form, Modal, Segment} from "./Theme";
-import {Checkbox, Dropdown} from "semantic-ui-react";
-import Grid from "semantic-ui-react/dist/commonjs/collections/Grid";
+import {Button, Checkbox, Grid, Message, Modal, Panel, Select} from "./ui";
import {monthNames} from "./Common";
-import Message from "semantic-ui-react/dist/commonjs/collections/Message";
export const dateRangeIsEmpty = (dateRange) => {
return dateRange[0] === null && dateRange[1] === null;
@@ -39,9 +36,10 @@ export function MonthsForm({monthsSelected, setMonthsSelected}) {
return {
+ size='lg'
+ onChange={(e) => {
// Add or remove month index if the respective checkbox is checked.
+ const checked = e.currentTarget.checked;
if (checked) {
setMonthsSelected([...monthsSelected, idx]);
} else {
@@ -51,36 +49,37 @@ export function MonthsForm({monthsSelected, setMonthsSelected}) {
/>
}
- return
+ const monthRowStyle = {display: 'grid', gridTemplateColumns: 'repeat(3, 1fr)', gap: '0.7em', marginBottom: '0.7em'};
+ const seasonRowStyle = {display: 'grid', gridTemplateColumns: 'repeat(4, 1fr)', gap: '0.7em'};
+
+ return
+
+
{monthCheckbox('January')}
+
{monthCheckbox('February')}
+
{monthCheckbox('March')}
+
+
+
{monthCheckbox('April')}
+
{monthCheckbox('May')}
+
{monthCheckbox('June')}
+
+
+
{monthCheckbox('July')}
+
{monthCheckbox('August')}
+
{monthCheckbox('September')}
+
+
+
{monthCheckbox('October')}
+
{monthCheckbox('November')}
+
{monthCheckbox('December')}
+
+
+ Winter
+ Spring
+ Summer
+ Fall
+
+
}
export function DateRangeForm({dateRange, setDateRange}) {
@@ -89,20 +88,16 @@ export function DateRangeForm({dateRange, setDateRange}) {
let yearRange = [];
for (let i = 1970; i <= currentYear; i++) {
- yearRange = [...yearRange, {
- key: i,
- text: i,
- value: i,
- }];
+ yearRange = [...yearRange, {value: i.toString(), label: i.toString()}];
}
- const handleFromYear = (e, {value}) => {
+ const handleFromYear = (value) => {
const toYear = dateRange[1] || currentYear;
- setDateRange([value, toYear]);
+ setDateRange([value ? parseInt(value) : null, toYear]);
}
- const handleToYear = (e, {value}) => {
- setDateRange([dateRange[0], value]);
+ const handleToYear = (value) => {
+ setDateRange([dateRange[0], value ? parseInt(value) : null]);
}
React.useEffect(() => {
@@ -113,37 +108,33 @@ export function DateRangeForm({dateRange, setDateRange}) {
}
}, [dateRange])
- const errorMessage = error && ;
+ const errorMessage = error && {error};
return
-
+
+
+ From Year
+
+
+
+ To Year
+
+
+
{errorMessage}
}
@@ -221,7 +212,7 @@ export function DateSelectorButton({
return Filter by Published Date
-
+
-
-
+
+
-
+
- Clear
- Close
+ Clear
+ Close
diff --git a/app/src/components/DesktopToggle.test.js b/app/src/components/DesktopToggle.test.js
index c1ffd806..4c393ded 100644
--- a/app/src/components/DesktopToggle.test.js
+++ b/app/src/components/DesktopToggle.test.js
@@ -1,5 +1,5 @@
import React from 'react';
-import {render, screen} from '@testing-library/react';
+import {render, screen} from '../test-utils';
import userEvent from '@testing-library/user-event';
import {DesktopToggle} from './Common';
@@ -34,7 +34,9 @@ describe('DesktopToggle', () => {
await userEvent.click(screen.getByTestId('toggle'));
// Not stopped yet; the Confirm modal is shown instead.
expect(mockSetDesktop).not.toHaveBeenCalled();
- expect(screen.getByText('Stop the desktop')).toBeInTheDocument();
+ // Mantine's Modal mounts its content on a subsequent render pass, so the
+ // text isn't present synchronously after the click.
+ expect(await screen.findByText('Stop the desktop')).toBeInTheDocument();
await userEvent.click(screen.getByText('Stop'));
expect(mockSetDesktop).toHaveBeenCalledWith(false);
@@ -43,7 +45,7 @@ describe('DesktopToggle', () => {
it('cancelling the confirmation leaves the desktop running', async () => {
render();
await userEvent.click(screen.getByTestId('toggle'));
- await userEvent.click(screen.getByText('Cancel'));
+ await userEvent.click(await screen.findByText('Cancel'));
expect(mockSetDesktop).not.toHaveBeenCalled();
expect(screen.getByTestId('toggle')).toBeChecked();
});
diff --git a/app/src/components/Docs.js b/app/src/components/Docs.js
index 344af300..815b1654 100644
--- a/app/src/components/Docs.js
+++ b/app/src/components/Docs.js
@@ -1,8 +1,7 @@
-import React, {useContext, useEffect, useState} from 'react';
+import React, {useEffect, useState} from 'react';
import {Link, Route, Routes, useLocation, useNavigate, useParams, useSearchParams} from "react-router";
-import {Grid, Icon as SIcon, Loader, StatisticLabel, StatisticValue} from "semantic-ui-react";
import {deleteFileGroups, getDocStatistics, tagFileGroup, untagFileGroup} from "../api";
-import {Media, ThemeContext} from "../contexts/contexts";
+import {Media} from "../contexts/contexts";
import {
APIButton,
BackButton,
@@ -18,13 +17,13 @@ import {
toLocaleString,
useTitle
} from "./Common";
-import {Button, darkTheme, Header, Icon, Segment, Statistic, Tab} from "./Theme";
+import {Button, ButtonGroup, Group, Header, Loading, Panel, Statistic, StatisticGroup, Tabs} from "./ui";
import {BulkTagModal} from "./BulkTagModal";
import {TaggedDeleteConfirmModal} from "./TaggedDeleteConfirmModal";
import {DeepSearchHint, docMimetypeFilterOptions, FilesView, SearchControlBar} from "./Files";
import {useAuthors, useDoc, useOneQuery, useSearchDocs, useSubjects} from "../hooks/customHooks";
import {TagsSelector} from "../Tags";
-import {toast} from "react-semantic-toasts-2";
+import {toast} from "./ui";
import {CollectionTable} from "./collections/CollectionTable";
import {AddToPlaylistButton} from "./AddToPlaylist";
import {CbzViewer} from "./CbzViewer";
@@ -84,19 +83,19 @@ function DocsPage() {
setSelectedDocs([]);
}
- const selectElm =
- {formatButtons &&
- {formatButtons}
- }
+ {/*
+ * The format picker and the actions are one row, separated by its gap.
+ *
+ * This IS a change: the picker used to sit 1em from the actions, held
+ * there by a span with `margin-right`, and it now sits at the row's
+ * 0.625rem like everything else in a row. The two groups read as slightly
+ * closer than before. Deliberate -- one spacing rule for buttons in a row
+ * beats a second one for buttons in a row that happen to be a picker --
+ * but it is a visual change and not a pure refactor.
+ */}
+
+ {formatButtons}
{actionButtons}
>;
})()}
-
+
-
+
-
+
-
+
+
+ About
+ Files
+
+ {aboutTab}
+ {filesTab}
+
-
-
-
- setSearchStr('')}
- onChange={setSearchStr}
- onSubmit={null}
- inputRef={searchInputRef}
- />
-
-
-
+ // No `marginBottom`: a top-level block of the page, spaced by the page's stack.
+ const header =
;
return <>
@@ -568,45 +559,24 @@ function DocsStatisticsPage() {
}, []);
if (statistics === null) {
- return ;
+ return ;
}
if (statistics === undefined) {
return Unable to fetch Doc Statistics;
}
- return
- Documents
-
-
- {toLocaleString(statistics.doc_count)}
- Documents
-
-
- {toLocaleString(statistics.epub_count)}
- eBooks
-
-
- {toLocaleString(statistics.pdf_count)}
- PDFs
-
-
- {toLocaleString(statistics.other_count)}
- Other
-
-
- {humanFileSize(statistics.total_size)}
- Total Size
-
-
- {toLocaleString(statistics.author_count)}
- Authors
-
-
- {toLocaleString(statistics.subject_count)}
- Subjects
-
-
- ;
+ return
+ Documents
+
+
+
+
+
+
+
+
+
+ ;
}
export function DocsRoute() {
diff --git a/app/src/components/DomainEditPage.test.js b/app/src/components/DomainEditPage.test.js
index 0a8ac9ae..e9fce0ca 100644
--- a/app/src/components/DomainEditPage.test.js
+++ b/app/src/components/DomainEditPage.test.js
@@ -95,7 +95,7 @@ describe('DomainEditPage', () => {
render();
- // Should show Semantic UI Loader with text
+ // Should show the Loading component with text
expect(screen.getByText(/loading domain/i)).toBeInTheDocument();
// Form should not be visible during initial load
diff --git a/app/src/components/DomainTagging.test.js b/app/src/components/DomainTagging.test.js
index 1c038ff2..354954c7 100644
--- a/app/src/components/DomainTagging.test.js
+++ b/app/src/components/DomainTagging.test.js
@@ -1,16 +1,17 @@
import React from 'react';
import {createMockDomain, createTestForm, render, screen} from '../test-utils';
+import {tagsContextFixture} from '../test-fixtures';
+import {TagsContext} from '../Tags';
import {CollectionEditForm} from './collections/CollectionEditForm';
import {DomainsPage} from './Archive';
-// Mock the TagsContext
-jest.mock('../Tags', () => ({
- TagsContext: {
- _currentValue: {
- SingleTag: ({name}) => {name}
- }
- },
-}));
+/*
+ * The Tags context used to be faked here as a bare object with a `_currentValue` and no
+ * `.Provider`, which worked only because `useContext` reads that React internal. It is now
+ * the real context carrying a fixture value, passed in by this spec rather than by the harness
+ * so that it is the same module instance the component reads -- see test-utils.js.
+ */
+const withTags = {contexts: [[TagsContext, tagsContextFixture()]]};
// Mock the DirectorySearch and DestinationForm components
jest.mock('./Common', () => ({
@@ -96,7 +97,8 @@ describe('Domain Tagging Logic', () => {
appliedTagName="News"
>
Child content
-
+ ,
+ withTags,
);
// Should render title
@@ -105,8 +107,8 @@ describe('Domain Tagging Logic', () => {
// Should render children
expect(screen.getByTestId('child-content')).toBeInTheDocument();
- // Should render applied tag
- expect(screen.getByTestId('applied-tag')).toHaveTextContent('News');
+ // The tag name a user would see, rather than a test id the old mock invented.
+ expect(screen.getByText('News')).toBeInTheDocument();
// Should render Save button
expect(screen.getByRole('button', {name: /save/i})).toBeInTheDocument();
@@ -148,7 +150,7 @@ describe('Domain Tagging Logic', () => {
);
- expect(screen.queryByTestId('applied-tag')).not.toBeInTheDocument();
+ expect(screen.queryByText('News')).not.toBeInTheDocument();
});
});
diff --git a/app/src/components/DonatePage.js b/app/src/components/DonatePage.js
index 59c32517..454ee6aa 100644
--- a/app/src/components/DonatePage.js
+++ b/app/src/components/DonatePage.js
@@ -1,7 +1,6 @@
import React, {useState} from "react";
import {PageContainer, useTitle} from "./Common";
-import {Button, Header, Modal, Segment, Table} from "./Theme";
-import {Icon, TableBody, TableCell, TableRow} from "semantic-ui-react";
+import {Button, Header, IconButton, Modal, Panel, Table} from "./ui";
import QRCode from "react-qr-code";
const bitcoinAddress = '1mi1ddLSd6LmkuwJd1ttZsKuED724vYjS';
@@ -17,18 +16,19 @@ function CoinQRButton({qrCodeValue, header, buttonColor}) {
const [open, setOpen] = useState(false);
return <>
- setOpen(true)}>
-
-
- setOpen(true)}/>
+ setOpen(false)}
onOpen={() => setOpen(true)}
>
{header}
-
+ {/* `media`: the white field a scanner needs is filtered as a unit by night mode,
+ rather than sitting on screen as a lit panel. */}
+
@@ -39,17 +39,17 @@ function CoinQRButton({qrCodeValue, header, buttonColor}) {
function CoinRow({header, qrCodeValue, address, buttonColor}) {
return <>
{header}
-
-
-
-
+
+
+
+
-
-
+
+
{address}
-
-
-
+
+
+
>
}
@@ -58,31 +58,28 @@ export function DonatePage() {
useTitle('Donate');
return
-
+ DonateWe appreciate any support you can provide to WROLPi!
+ >
+ Donate on PayPal
+
-
+
;
}
diff --git a/app/src/components/Download.cy.js b/app/src/components/Download.cy.js
index 39195b35..9d57281d 100644
--- a/app/src/components/Download.cy.js
+++ b/app/src/components/Download.cy.js
@@ -1,6 +1,21 @@
import React from 'react';
import {ArchiveDownloadForm, ChannelDownloadForm, EditRSSDownloadForm} from './Download';
-import 'semantic-ui-offline/semantic.min.css';
+
+/*
+ * A field in the error state.
+ *
+ * The old form controls took `error` and put the bare word on the DOM node, so a spec could
+ * assert `have.attr 'error'`. Our fields mark an invalid control the way assistive
+ * technology reads it instead -- `aria-invalid` on the input. The old assertion did not
+ * fail when the controls changed; it went on looking for an attribute that no longer
+ * exists, on a field that is both styled and announced as invalid.
+ *
+ * Valid is `aria-invalid="false"`, not a missing attribute: the field states its validity
+ * either way, which is what lets a screen reader say a control is fine rather than say
+ * nothing about it. Both are asserted by value so neither direction can pass on absence.
+ */
+const shouldBeInvalid = (selector) => cy.get(selector).should('have.attr', 'aria-invalid', 'true');
+const shouldBeValid = (selector) => cy.get(selector).should('have.attr', 'aria-invalid', 'false');
describe('', () => {
beforeEach(() => {
@@ -9,18 +24,18 @@ describe('', () => {
it('displays error for invalid URL', () => {
cy.get('#urls_textarea').type('invalid-url').wait(500);
- cy.get('#urls_textarea').should('have.attr', 'error');
+ shouldBeInvalid('#urls_textarea');
});
it('enables download button for valid URL', () => {
cy.get('#urls_textarea').type('https://wrolpi.org').wait(500);
- cy.get('#urls_textarea').should('not.have.attr', 'error');
+ shouldBeValid('#urls_textarea');
cy.get('#download_form_download_button').should('be.visible').and('not.have.attr', 'disabled');
});
it('disables download button for invalid URL', () => {
cy.get('#urls_textarea').type('invalid-url').wait(500);
- cy.get('#urls_textarea').should('have.attr', 'error');
+ shouldBeInvalid('#urls_textarea');
cy.get('#download_form_download_button').should('be.visible').and('have.attr', 'disabled');
});
@@ -30,11 +45,11 @@ describe('', () => {
it('clears error when switching from invalid to valid URL', () => {
cy.get('#urls_textarea').type('invalid-url').wait(500);
- cy.get('#urls_textarea').should('have.attr', 'error');
+ shouldBeInvalid('#urls_textarea');
cy.get('#urls_textarea').clear();
cy.get('#urls_textarea').type('https://wrolpi.org').wait(500);
- cy.get('#urls_textarea').should('not.have.attr', 'error');
+ shouldBeValid('#urls_textarea');
});
});
@@ -54,16 +69,42 @@ describe('', () => {
it('displays error for invalid URL', () => {
cy.get('#url_input').type('invalid-url').wait(500);
- cy.get('#url_input').should('have.attr', 'error');
+ shouldBeInvalid('#url_input');
});
it('can choose different options', () => {
- cy.get('#download_frequency_selector').click()
- .get('.item').contains('Once').click();
- cy.get('#download_frequency_selector').click()
- .get('.item').contains('Daily').click();
- cy.get('#download_frequency_selector').click()
- .get('.item').contains('180 Days').click();
+ /*
+ * `#download_frequency_selector` and `.item` named the old dropdown and its menu rows.
+ * SelectField ids each control `_select`, and an option carries the ARIA role
+ * rather than a class -- which is what a screen reader navigates, so it is the better
+ * handle of the two.
+ */
+ const chooseFrequency = (label) => {
+ cy.get('#frequency_select').click();
+ /*
+ * One query, so Cypress retries the whole thing while the dropdown opens.
+ * `cy.get(...).contains(...)` does not: `get` settles on whatever matched at
+ * that instant, and the options are not in the DOM yet when it does.
+ *
+ * `:visible` because the form carries several Selects, each keeping its options
+ * mounted while closed -- a bare `[role="option"]` matches a hidden one from a
+ * dropdown that was never opened, and clicking that fails.
+ */
+ cy.get('[role="option"]:visible').should('have.length.greaterThan', 0);
+ /*
+ * `force` because the dropdown re-renders as its open transition settles, and
+ * Cypress aborts a click whose element was replaced between locating it and
+ * clicking it. Visibility is asserted on its own above, so this is not
+ * skipping a check that could hide an unreachable control.
+ */
+ cy.contains('[role="option"]:visible', label).should('be.visible')
+ .click({force: true});
+ cy.get('#frequency_select').should('have.value', label);
+ };
+
+ chooseFrequency('Once');
+ chooseFrequency('Daily');
+ chooseFrequency('180 Days');
});
});
@@ -96,8 +137,14 @@ describe('', () => {
tag_names: ['Automotive'],
url: "https://www.youtube.com/feeds/videos.xml?channel_id=UC4t8bw1besFTyjW7ZBCOIrw"
};
- cy.mountWithTags();
- cy.intercept('GET', '/api/downloaders', downloadersResponse).as('downloaders');
+ /*
+ * The intercepts are declared BEFORE the mount, which is the whole reason this
+ * block is ordered this way. Mounting first lets the form's own effects fire
+ * against a bare network -- `/api/downloaders` rejected with "Failed to fetch",
+ * and Cypress fails a test on an unhandled rejection out of application code
+ * whatever it was in the middle of asserting.
+ */
+ cy.intercept('GET', '**/api/downloaders', downloadersResponse).as('downloaders');
cy.intercept('POST', '**/api/files/search_directories', {
statusCode: 200,
body: {
@@ -106,47 +153,64 @@ describe('', () => {
},
}).as('searchDirectories');
+ cy.mountWithTags();
});
it('can edit videos download', () => {
// Weekly frequency.
cy.get('#url_input').should('have.value',
'https://www.youtube.com/feeds/videos.xml?channel_id=UC4t8bw1besFTyjW7ZBCOIrw');
- cy.get('div.divider.text').contains('Weekly');
+ // The old dropdown showed its choice in a `div.divider.text`; a Select is a real input
+ // holding the label, so the value can be read rather than searched for.
+ cy.get('#frequency_select').should('have.value', 'Weekly');
cy.get('input[name="title_include"]').should('have.value', 'included');
cy.get('input[name="title_exclude"]').should('have.value', 'excluded');
cy.get('#destination_search_form').should('be.visible');
- cy.get('div.ui.large.label').contains('Automotive').should('be.visible');
+ cy.get('.wrolpi-tag').contains('Automotive').should('be.visible');
});
it('can submit videos download', () => {
cy.get('button[type="submit"]').should('be.visible');
- cy.intercept('PUT', '/api/download/1', (req) => {
- expect(req.body).to.deep.equal(JSON.stringify({
- "destination": "destination directory",
- "downloader": "rss",
- "frequency": 604800,
- "settings": {
- "excluded_urls": null,
- "title_exclude": "excluded",
- "title_include": "included",
- "video_resolutions": [
- "720p",
- "480p",
- "maximum"
- ],
- "video_format": "mp4",
- "minimum_duration": null,
- "maximum_duration": null
- },
- "sub_downloader": "video",
- "tag_names": ["Automotive"],
- "urls": ["https://www.youtube.com/feeds/videos.xml?channel_id=UC4t8bw1besFTyjW7ZBCOIrw"]
- }));
- req.reply({statusCode: 204});
- }).as('downloadPut');
+ cy.intercept('PUT', '/api/download/1', {statusCode: 204}).as('downloadPut');
cy.get('button[type="submit"]').click();
+
+ /*
+ * The edit form sends the download back unchanged.
+ *
+ * This asserted one exact JSON string, which broke the moment the video downloader
+ * gained settings the form fills in with their defaults -- `compress_singlefile`,
+ * `audio_only`, `user_agent` and five more. None of that is the subject: what
+ * matters is that the values the user was shown are the values that go back.
+ *
+ * So the envelope is still matched exactly, which catches a field appearing or
+ * vanishing at the top level, while `settings` is checked for the seven the fixture
+ * set. Waiting on the alias matters as much as either -- without it the click ended
+ * the test and an interceptor that never ran would have passed silently.
+ */
+ cy.wait('@downloadPut').its('request.body').then((body) => {
+ const sent = typeof body === 'string' ? JSON.parse(body) : body;
+ const {settings, ...envelope} = sent;
+
+ expect(envelope).to.deep.equal({
+ destination: 'destination directory',
+ downloader: 'rss',
+ frequency: 604800,
+ sub_downloader: 'video',
+ tag_names: ['Automotive'],
+ urls: ['https://www.youtube.com/feeds/videos.xml?channel_id=UC4t8bw1besFTyjW7ZBCOIrw'],
+ });
+
+ expect(settings).to.deep.include({
+ excluded_urls: null,
+ title_exclude: 'excluded',
+ title_include: 'included',
+ video_resolutions: ['720p', '480p', 'maximum'],
+ video_format: 'mp4',
+ minimum_duration: null,
+ maximum_duration: null,
+ });
+ });
});
});
diff --git a/app/src/components/Download.js b/app/src/components/Download.js
index a7af9306..db1f1478 100644
--- a/app/src/components/Download.js
+++ b/app/src/components/Download.js
@@ -7,23 +7,18 @@ import {
InfoPopup,
mergeDeep,
RequiredAsterisk,
- useLocalStorage
+ SimpleAccordion,
+ Toggle,
+ useLocalStorage,
+ validURL,
+ validURLs,
} from "./Common";
-import Icon from "semantic-ui-react/dist/commonjs/elements/Icon";
-import Message from "semantic-ui-react/dist/commonjs/collections/Message";
-import {Accordion, Button, Form, Header} from "./Theme";
-import {Form as SForm, FormDropdown} from "semantic-ui-react";
+import {Button, Group, Header, Icon, Message, MultiSelect, NumberInput, Select, Stack, TextInput, Textarea} from "./ui";
import {Link} from "react-router";
import {TagsSelector} from "../Tags";
-import Grid from "semantic-ui-react/dist/commonjs/collections/Grid";
import {
commaSeparatedValidator,
- InputForm,
- NumberInputForm,
- ToggleForm,
- UrlInput,
- UrlsTextarea,
- useForm
+ useForm,
} from "../hooks/useForm";
import {
channelFrequencyOptions,
@@ -41,8 +36,134 @@ import {
weeklyOption
} from "./Vars";
+/*
+ * Download.js was built around the old
}
@@ -925,108 +898,67 @@ export function ChannelDownloadForm({
}
}, [form.formData]);
- const onceMessage =
- Download Once
- You have selected a frequency of Once, this is useful when you want to download
- all videos in a Playlist, and when you do not want to download any videos added to the playlist
- in the future.
+ const onceMessage =
+ You have selected a frequency of Once, this is useful when you want to download
+ all videos in a Playlist, and when you do not want to download any videos added to the playlist
+ in the future.
;
// Default to "new" download buttons.
actions = actions || DownloadFormButtons;
const actionsElm = actions({onDelete, onCancel, form});
- return