Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

feat(dashboard): allow user-defined dashboard card sizes #796

Merged
merged 15 commits into from
Jan 13, 2023

Conversation

maxcao13
Copy link
Member

@maxcao13 maxcao13 commented Jan 7, 2023

Signed-off-by: Max Cao macao@redhat.com

Welcome to Cryostat! 👋

Before contributing, make sure you have:

  • Read the contributing guidelines
  • Linked a relevant issue which this PR resolves
  • Linked any other relevant issues, PR's, or documentation, if any
  • Resolved all conflicts, if any
  • Rebased your branch PR on top of the latest upstream main branch
  • Attached at least one of the following labels to the PR: [chore, ci, docs, feat, fix, test]
  • Signed the last commit: git commit --amend --signoff

Fixes: #729

Description of the change:

Allows cards to be resizable and for these configs to be persisted.

Motivation for the change:

See #729

How to manually test:

  1. Run normally and add various card configs.
  2. Resize some cards by dragging the right side of them.
  3. Notice the sizes are persisted when reloading or navigating elsewhere.

@maxcao13 maxcao13 added the feat New feature or request label Jan 7, 2023
@maxcao13 maxcao13 requested review from andrewazores and tthvo January 7, 2023 03:09
@mergify mergify bot added the safe-to-test label Jan 7, 2023
@maxcao13 maxcao13 changed the title init commit feat(dashboard): allow user-defined dashboard card sizes Jan 7, 2023
@github-actions
Copy link

github-actions bot commented Jan 7, 2023

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-796-b613b493defe7267b56cf03df2fe1ea6a94c78fe sh smoketest.sh

@maxcao13
Copy link
Member Author

maxcao13 commented Jan 7, 2023

Ah, I assume the placeholders won't be up to test with since the test image won't have DEVELOPMENT features enabled. I will set the Automated Analysis Card default span to 6 to show the functionality then.

@github-actions
Copy link

github-actions bot commented Jan 7, 2023

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-796-f65ecf43dedc0c8c08844b8f358bb36e04832408 sh smoketest.sh

@andrewazores
Copy link
Member

andrewazores commented Jan 9, 2023

Ah, I assume the placeholders won't be up to test with since the test image won't have DEVELOPMENT features enabled. I will set the Automated Analysis Card default span to 6 to show the functionality then.

You can use the PR CI test images and have development feature flags enabled:

$ CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-796-f65ecf43dedc0c8c08844b8f358bb36e04832408  CRYOSTAT_CORS_ORIGIN=http://localhost:9000 CRYOSTAT_DISABLE_SSL=true sh smoketest.sh
$ #in new terminal:
$ cd cryostat-web
$ yarn start:dev

Obviously for -web PRs in particular, you need to also check out the PR in the -web repo, which limits the utility of these steps. But it's possible in general and could be useful if testing a backend change that affects a flagged component.

@andrewazores
Copy link
Member

Wondering if this issue wanted a resizable card like a draggable resize bar (e.g. the DrawerContentBody ?). If so, it may be possible to implement this but will take some time as I would have to take a look a look at Patternfly's isResizable implementation.

Something like a draggable resize bar would be nice, or any other method that allows the user some fine-grained control. It could simply be a slider component that they can pull up somehow which allows value selection from 1-12 for the card span, for example. Though, I'm not sure if all of those sizes really make sense - is there a reason for the user to choose a 1 or 11 span, or 2 or 10? Maybe if they are using an ultrawide aspect monitor?

@andrewazores
Copy link
Member

diff --git a/src/app/Dashboard/Dashboard.tsx b/src/app/Dashboard/Dashboard.tsx
index 836929f..e84cc30 100644
--- a/src/app/Dashboard/Dashboard.tsx
+++ b/src/app/Dashboard/Dashboard.tsx
@@ -226,8 +226,8 @@ export const Dashboard: React.FC<DashboardProps> = (_) => {
   );
 
   const handleResize = React.useCallback(
-    (idx: number) => {
-      dispatch(dashboardCardConfigResizeCardIntent(idx, cardConfigs[idx].span === 12 ? 6 : 12));
+    (idx: number, span: gridSpans) => {
+      dispatch(dashboardCardConfigResizeCardIntent(idx, span));
     },
     [dispatch, cardConfigs]
   );
@@ -245,7 +245,7 @@ export const Dashboard: React.FC<DashboardProps> = (_) => {
                   idx={idx}
                   defaultSpan={getConfigByName(cfg.name).defaultSpan}
                   onRemove={() => handleRemove(idx)}
-                  onResize={() => handleResize(idx)}
+                  onResize={(span: number) => handleResize(idx, span)}
                 />,
               ],
             })}
diff --git a/src/app/Dashboard/DashboardCardActionMenu.tsx b/src/app/Dashboard/DashboardCardActionMenu.tsx
index 04cee70..febd0c2 100644
--- a/src/app/Dashboard/DashboardCardActionMenu.tsx
+++ b/src/app/Dashboard/DashboardCardActionMenu.tsx
@@ -37,7 +37,16 @@
  */
 import { CardConfig } from '@app/Shared/Redux/Configurations/DashboardConfigSlicer';
 import { RootState } from '@app/Shared/Redux/ReduxStore';
-import { Dropdown, DropdownItem, gridSpans, KebabToggle } from '@patternfly/react-core';
+import {
+  Dropdown,
+  DropdownItem,
+  gridSpans,
+  KebabToggle,
+  Popover,
+  Slider,
+  Text,
+  TextVariants,
+} from '@patternfly/react-core';
 import * as React from 'react';
 import { useTranslation } from 'react-i18next';
 import { useSelector } from 'react-redux';
@@ -46,12 +55,13 @@ export interface DashboardCardActionProps {
   idx: number;
   defaultSpan: gridSpans;
   onRemove: () => void;
-  onResize: () => void;
+  onResize: (span: gridSpans) => void;
 }
 
 export const DashboardCardActionMenu: React.FunctionComponent<DashboardCardActionProps> = (props) => {
   const cardConfigs: CardConfig[] = useSelector((state: RootState) => state.dashboardConfigs.list);
   const [isOpen, setOpen] = React.useState(false);
+  const [span, setSpan] = React.useState(props.defaultSpan || 12);
 
   const [t] = useTranslation();
 
@@ -62,9 +72,23 @@ export const DashboardCardActionMenu: React.FunctionComponent<DashboardCardActio
     [setOpen]
   );
 
+  const handleSpanChange = React.useCallback(
+    (v) => {
+      if (v < 1) {
+        v = 1;
+      }
+      if (v > 12) {
+        v = 12;
+      }
+      setSpan(v);
+    },
+    [setSpan]
+  );
+
   return (
     <>
       <Dropdown
+        onBlur={() => props.onResize(span)}
         isPlain
         isFlipEnabled
         menuAppendTo={'parent'}
@@ -76,13 +100,11 @@ export const DashboardCardActionMenu: React.FunctionComponent<DashboardCardActio
           <DropdownItem key="Remove" onClick={props.onRemove}>
             {t('REMOVE', { ns: 'common' })}
           </DropdownItem>,
-          props.defaultSpan == 12 ? null : (
-            <DropdownItem key="Resize" onClick={props.onResize}>
-              {cardConfigs[props.idx].span === props.defaultSpan
-                ? t('EXPAND', { ns: 'common' })
-                : t('DashboardCardActionMenu.SHRINK', { val: props.defaultSpan })}
-            </DropdownItem>
-          ),
+          <DropdownItem key="Resize">
+            <Text component={TextVariants.p}>{t('EXPAND', { ns: 'common' })}</Text>
+            <Slider value={span} onChange={handleSpanChange} step={1} min={1} max={12} />
+            <Text component={TextVariants.p}>{span}</Text>
+          </DropdownItem>,
         ].filter((item) => item !== null)}
       />
     </>

Quick hack on top of your patch here to demonstrate. It's not a great UX, but it gives some easy flexibility to lay things out how the user might like it depending on the content they choose to place and what kind of display and display settings they use.

image

@tthvo
Copy link
Member

tthvo commented Jan 10, 2023

Just a though to extend above concept: maybe we could allow dragging at corners (or at least bottom-right) of any card to resize instead of slider. I guess we can make it "snap" to certain defined span if it reaches a certain size. Not sure how hard it is to implement it :D

@andrewazores
Copy link
Member

Yea I agree with that, I think that's similar to what we were mentioning above with the "draggable resize". I think in that case the card should always snap to the span width sizes as it is dragged. I also have no idea how hard this would be to implement though, not sure if there is already any support in Patternfly for something like this.

@github-actions
Copy link

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-796-f65ecf43dedc0c8c08844b8f358bb36e04832408 sh smoketest.sh

@andrewazores
Copy link
Member

Anyway, I don't think we need to go all the way to drag-to-resize card borders with this initial PR. Something better than my hack above hopefully, but at least something still that allows the user to pick a gridSpans (1-12) value for each card, just with nicer controls than what I did.

@maxcao13 maxcao13 force-pushed the user-defined-card-sizes branch from f65ecf4 to a86ae85 Compare January 11, 2023 23:27
@github-actions
Copy link

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-796-a86ae85a63585a4c53bd2ac89c1fa694c7c8d913 sh smoketest.sh

@maxcao13
Copy link
Member Author

Updated the resizing, thoughts?

@github-actions
Copy link

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-796-f0ef43dc6cd2362048fe40346d88aeea0dfd713f sh smoketest.sh

Copy link
Member

@andrewazores andrewazores left a comment

Choose a reason for hiding this comment

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

Nice, this looks pretty decent and works quite well.

src/app/Dashboard/DraggableRef.tsx Outdated Show resolved Hide resolved
@github-actions
Copy link

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-796-c608d792edd11dc1cd29a6ff7c43663548154c3d sh smoketest.sh

@github-actions
Copy link

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-796-86901837d671f1daef16e5619f5c154e1ed8a355 sh smoketest.sh

@maxcao13 maxcao13 force-pushed the user-defined-card-sizes branch from 8690183 to 8b82fa6 Compare January 13, 2023 18:04
@maxcao13
Copy link
Member Author

I think maybe I will hold off on tests like with the other beta-ish Dashboard features, as this may be changed in the future.

@github-actions
Copy link

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-796-8b82fa609e2aadb58e01693935d9c5c9a025b6a6 sh smoketest.sh

@github-actions
Copy link

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-796-d8f08b3863b81b77caf81870c2cec55c8ea56e94 sh smoketest.sh

@github-actions
Copy link

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-796-9c5c7d737c86dd9fbbe3e3f312f7b6281c9de64f sh smoketest.sh

Copy link
Member

@andrewazores andrewazores left a comment

Choose a reason for hiding this comment

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

Looks great to me.

@andrewazores andrewazores merged commit dcbb82e into cryostatio:main Jan 13, 2023
@maxcao13
Copy link
Member Author

Oh, there was actually still a bug with resizing the cards over the right side of the screen that I missed. I will file a quick PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[Task] Extend Dashboard layout to grid with user-defined card sizes
3 participants