Skip to content

ThreatsDataViews: Improve responsiveness #40670

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.

Sign up for GitHub

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

Merged
dkmyta merged 11 commits into from
Jan 14, 2025
Merged

ThreatsDataViews: Improve responsiveness #40670

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
09991b1
Init project branch
nateweller Nov 14, 2024
9aa31c3
Init project branch
nateweller Nov 14, 2024
31c6f81
Improve component responsiveness
dkmyta Dec 18, 2024
2f73aa8
Init project branch
nateweller Nov 14, 2024
eadeae6
Fix merge conflict
nateweller Dec 31, 2024
ebe19c3
Simplify, and only default to list view in mobile
dkmyta Jan 2, 2025
cc1e0e9
Fix spacing
dkmyta Jan 2, 2025
79fbdcc
Minor adjustments to initialization of defaultLayouts, remove memo
nateweller Jan 12, 2025
2494779
Remove style overrides on core component
nateweller Jan 12, 2025
009f5ef
Rebase, fix conflicts
dkmyta Jan 14, 2025
d63c91f
Reapply min-width for ScanToggleGroupControl
dkmyta Jan 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Improve component responsiveness
  • Loading branch information
@dkmyta @nateweller
dkmyta authored and nateweller committed Jan 12, 2025
commit 31c6f816f772c350fd20e9eee102bd3be554a003
78 changes: 52 additions & 26 deletions projects/js-packages/components/components/threats-data-views/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@ import {
import { dateI18n } from '@wordpress/date';
import { __ } from '@wordpress/i18n';
import { Icon } from '@wordpress/icons';
import { useCallback, useMemo, useState } from 'react';
import { useCallback, useEffect, useMemo, useState } from 'react';
import Badge from '../badge';
import useBreakpointMatch from '../layout/use-breakpoint-match';
import ThreatFixerButton from '../threat-fixer-button';
import ThreatSeverityBadge from '../threat-severity-badge';
import {
Expand Down Expand Up @@ -79,16 +80,7 @@ export default function ThreatsDataViews( {
onIgnoreThreats?: ActionButton< Threat >[ 'callback' ];
onUnignoreThreats?: ActionButton< Threat >[ 'callback' ];
} ): JSX.Element {
const baseView = {
sort: {
field: 'severity',
direction: 'desc' as SortDirection,
},
search: '',
filters: filters || [],
page: 1,
perPage: 20,
};
const [ isSm ] = useBreakpointMatch( [ 'sm', 'lg' ], [ null, '<' ] );

/**
* DataView default layouts.
Expand All @@ -97,15 +89,19 @@ export default function ThreatsDataViews( {
*
* @see https://developer.wordpress.org/block-editor/reference-guides/packages/packages-dataviews/#defaultlayouts-record-string-view
*/
const defaultLayouts: SupportedLayouts = {
table: {
...baseView,
fields: [ THREAT_FIELD_SEVERITY, THREAT_FIELD_TYPE, THREAT_FIELD_AUTO_FIX ],
titleField: THREAT_FIELD_TITLE,
descriptionField: THREAT_FIELD_DESCRIPTION,
showMedia: false,
},
list: {
const defaultLayouts: SupportedLayouts = useMemo( () => {
const baseView = {
sort: {
field: 'severity',
direction: 'desc' as SortDirection,
},
search: '',
filters: filters || [],
page: 1,
perPage: 20,
};

const listLayout = {
...baseView,
fields: [
THREAT_FIELD_SEVERITY,
Expand All @@ -116,18 +112,48 @@ export default function ThreatsDataViews( {
titleField: THREAT_FIELD_TITLE,
mediaField: THREAT_FIELD_ICON,
showMedia: true,
},
};
};

const tableLayout = {
...baseView,
fields: [ THREAT_FIELD_SEVERITY, THREAT_FIELD_TYPE, THREAT_FIELD_AUTO_FIX ],
titleField: THREAT_FIELD_TITLE,
descriptionField: THREAT_FIELD_DESCRIPTION,
showMedia: false,
};

return isSm ? { list: listLayout } : { table: tableLayout, list: listLayout };
Copy link
Contributor

@nateweller nateweller Dec 31, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary to prevent mobile users from accessing the table layout like this? It would be nice if we could default the view to list on mobile, but still keep it accessible.

Then we would only need to worry about setting the list vs table layout on first render:

const [view, setView] = useState( isSm ? listView : tableView )

Assuming isSm works out of the box without a re-render. Otherwise we could use the window directly.

Anyways, all IMO.

Copy link
Contributor Author

@dkmyta dkmyta Jan 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary to prevent mobile users from accessing the table layout like this? It would be nice if we could default the view to list on mobile, but still keep it accessible.

Ah my apologies, I must have misunderstood - I was under the impression we wanted to force list view but defaulting and allowing users to switch to table view makes sense to me, will adjust 👍🏻

Copy link
Contributor Author

@dkmyta dkmyta Jan 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming isSm works out of the box without a re-render. Otherwise we could use the window directly.

@nateweller Out of curiosity, ideally how would you expect this function? Using just useBreakpointMatch might be fine, it does require a re-render but maybe thats not an issue. For instance, if I am in table view in full screen and shrink my screen to the trigger point, would I really want the UI to update to list view (and vice-versa). Maybe its okay just have the default on re-render which strictly apply to mobile users. Happy to look in to the window approach, just thought I'd run it by you first. Here's the first take - 2f5e469

Also noting that with the addition of the extra settings toggle in the header, things are now a little squished:
Screen Shot on 2025-01-02 at 09-47-41

What do you think if we adding alternate styling/content for mobile for the ToggleGroupControl? Perhaps:
Screen Shot on 2025-01-02 at 09-45-42

Noting that we may need to take the window approach if we go that route.

}, [ filters, isSm ] );

const tableView: View = useMemo(
() => ( {
type: 'table',
...defaultLayouts.table,
} ),
[ defaultLayouts.table ]
);

const listView: View = useMemo(
() => ( {
type: 'list',
...defaultLayouts.list,
} ),
[ defaultLayouts.list ]
);

/**
* DataView view object - configures how the dataset is visible to the user.
*
* @see https://developer.wordpress.org/block-editor/reference-guides/packages/packages-dataviews/#view-object
*/
const [ view, setView ] = useState< View >( {
type: 'table',
...defaultLayouts.table,
} );
const [ view, setView ] = useState< View >( tableView );

/**
* Set the initial view based on the screen size.
*/
useEffect( () => {
setView( isSm ? listView : tableView );
}, [ isSm, listView, tableView ] );

/**
* Compute values from the provided threats data.
Expand Down
1 change: 1 addition & 0 deletions ...packages/components/components/threats-data-views/threats-status-toggle-group-control.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ export default function ThreatsStatusToggleGroupControl( {
*/
const isStatusFilterSelected = useMemo(
() => ( threatStatuses: ThreatStatus[] ) =>
Array.isArray( view.filters ) &&
view.filters.some(
filter =>
filter.field === 'status' &&
Expand Down
7 changes: 6 additions & 1 deletion projects/plugins/protect/src/js/routes/home/styles.module.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
padding-left: 0;
padding-right: 0;
overflow: hidden;

> * {
margin-left: calc( var( --spacing-base ) * -3 ); // -24px
margin-right: calc( var( --spacing-base ) * -3 ); // -24px
Expand Down Expand Up @@ -68,4 +68,9 @@
.stat-card-icon {
margin-bottom: 0;
}

.scan-report-container > * {
margin-left: 0;
margin-right: 0;
}
}
22 changes: 22 additions & 0 deletions projects/plugins/protect/src/js/routes/scan/styles.module.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,15 @@
.scan-results-container {
:global {
@media ( max-width: 599px ) {
div.dataviews-wrapper .dataviews__view-actions {
flex-wrap: wrap;
justify-content: center;
gap: calc( var( --spacing-base ) * 1.5 ); // 12px;
}
}
}
}

.hero-main {
max-width: 512px;
}
Expand Down Expand Up @@ -33,3 +45,13 @@
.last-checked {
width: fit-content;
}
<<<<<<< HEAD
=======

@media ( max-width: 599px ) {
.scan-results-container > * {
margin-left: 0;
margin-right: 0;
}
}
>>>>>>> 12d3c25056 (Improve component responsiveness)