add Guided tour tile #5822

abhinandan13jan · 2020-06-25T11:25:20Z

Adresses

https://issues.redhat.com/browse/ODC-4131
This must go in after -- #5764

Issue

Add guided tour discoverability to dev console

Solution

Created GuidedTourTile component and added to AddActions

ScreenShot

Tests

Added GuidedTourTile specs

Browser Conformation

Chrome

christianvogt · 2020-06-25T19:12:11Z

vs

Too much white space above and below header
Need to add kebab menu to Remove Guided Tours
Mis-alignment of list items with title
Footer line separator does not span full width

You may want to use a custom Card instead of CatalogTile because this is a very different tile than the rest.

abhinandan13jan · 2020-06-30T16:18:38Z

vs

Too much white space above and below header

Need to add kebab menu to Remove Guided Tours

Mis-alignment of list items with title

Footer line separator does not span full width

You may want to use a custom Card instead of CatalogTile because this is a very different tile than the rest.

@christianvogt Updated to bring it closer to UX design, added Kebab. The UX font size is different than that used in rest of gallery

christianvogt · 2020-06-30T20:55:01Z

frontend/packages/dev-console/src/components/GuidedTourTile.tsx

+              <Text component={TextVariants.h3}> Guided Tours</Text>
+            </FlexItem>
+            <FlexItem breakpointMods={[{ modifier: FlexModifiers['align-right'] }]}>
+              <Kebab options={[{ label: 'remove guided tours', callback: () => null }]} />


Use CardActions component. See PF examples: https://patternfly-react.surge.sh/documentation/react/components/card

christianvogt · 2020-06-30T20:55:17Z

frontend/packages/dev-console/src/components/GuidedTourTile.tsx

+        <CardBody className="odc-guidedtour-tile__body">
+          {orderedTours.map((tour) => (
+            <div key={tour.name} className="odc-guidedtour-tile__tour">
+              <Button variant="link">{tour.name}</Button>


Use react-router Link

christianvogt · 2020-06-30T20:55:27Z

frontend/packages/dev-console/src/components/GuidedTourTile.tsx

+          ))}
+        </CardBody>
+        <CardFooter className="odc-guidedtour-tile__footer">
+          <Button


use isInline button

Use react-router Link

christianvogt · 2020-06-30T20:59:27Z

frontend/packages/dev-console/src/components/GuidedTourTile.tsx

+        <CardHeader>
+          <Flex className="odc-guidedtour-tile__title">
+            <FlexItem>
+              <Text component={TextVariants.h3}> Guided Tours</Text>


Use CardTitle. See PF examples: https://patternfly-react.surge.sh/documentation/react/components/card

CardTitle seems to be unavailable with current PF

The PF v4 PR is on its way. Maybe just wait for it to merge otherwise we'll have to change this again as soon as it's in.
Or look at the previous release docs and find common components between the two that works.

christianvogt · 2020-07-06T17:17:22Z

@abhinandan13jan needs rebase. Also you can use new pf components now.

abhinandan13jan · 2020-07-07T13:25:51Z

@christianvogt I've rebased and made changes to accomodate CardHeaderMain. The CardTitle component is causing misalignments even with display:inline

christianvogt · 2020-07-07T17:24:21Z

frontend/packages/dev-console/src/components/GuidedTourTile.tsx

+    <GalleryItem className="odc-guidedtour-tile">
+      <Card className="odc-guidedtour-tile__card">
+        <CardHeader>
+          <CardHeaderMain className="odc-guidedtour-tile__title">Guided Tours</CardHeaderMain>


Suggested change

<CardHeaderMain className="odc-guidedtour-tile__title">Guided Tours</CardHeaderMain>

<CardHeaderMain>

<Title headingLevel="h1" size="xl">

Guided Tours

</Title>

</CardHeaderMain>

christianvogt · 2020-07-07T17:26:13Z

frontend/packages/dev-console/src/components/GuidedTourTile.scss

+  &__title {
+    font-weight: var(--pf-global--FontWeight--bold);
+    font-size: var(--pf-global--FontSize--lg);
+  }


You can use a patternfly Title component instead.

Suggested change

&__title {

font-weight: var(--pf-global--FontWeight--bold);

font-size: var(--pf-global--FontSize--lg);

}

christianvogt · 2020-07-07T17:28:34Z

frontend/packages/dev-console/src/components/GuidedTourTile.scss

+  }
+  &__footer {
+    border-top: 1px solid var(--pf-global--BorderColor--100);
+    padding: var(--pf-global--spacer--sm) var(--pf-global--spacer--lg);


Suggested change

padding: var(--pf-global--spacer--sm) var(--pf-global--spacer--lg);

padding-top: var(--pf-global--spacer--sm);

padding-bottom: var(--pf-global--spacer--sm);

christianvogt · 2020-07-07T17:30:40Z

frontend/packages/dev-console/src/components/GuidedTourTile.scss

+    text-align: left;
+  }
+  &__arrowbtn {
+    margin-right: var(--pf-global--spacer--md);


Suggested change

margin-right: var(--pf-global--spacer--md);

margin-right: var(--pf-global--spacer--sm);

christianvogt · 2020-07-07T17:31:51Z

frontend/packages/dev-console/src/components/GuidedTourTile.tsx

+            />
+          </CardActions>
+        </CardHeader>
+        <CardBody className="odc-guidedtour-tile__body">


Suggested change

<CardBody className="odc-guidedtour-tile__body">

<CardBody>

christianvogt · 2020-07-07T17:32:38Z

frontend/packages/dev-console/src/components/GuidedTourTile.scss

+    padding: var(--pf-global--spacer--sm) var(--pf-global--spacer--md);
+    text-align: left;


Suggested change

padding: var(--pf-global--spacer--sm) var(--pf-global--spacer--md);

text-align: left;

margin-bottom: var(--pf-global--spacer--sm);

christianvogt · 2020-07-07T17:32:54Z

frontend/packages/dev-console/src/components/GuidedTourTile.scss

+  &__header {
+    padding: 0px var(--pf-global--spacer--md);
+    margin: 0px;
+  }
+  &__body {
+    padding: var(--pf-global--spacer--sm);
+  }


Suggested change

&__header {

padding: 0px var(--pf-global--spacer--md);

margin: 0px;

}

&__body {

padding: var(--pf-global--spacer--sm);

}

christianvogt · 2020-07-07T17:35:31Z

frontend/packages/dev-console/src/components/GuidedTourTile.tsx

+  const actionDropdownItem = [<DropdownItem key="link">Remove guided tours</DropdownItem>];
+  const orderedTours = tours.length > 3 ? tours.slice(0, 3) : tours;
+  return (
+    <GalleryItem className="odc-guidedtour-tile">


Suggested change

<GalleryItem className="odc-guidedtour-tile">

<GalleryItem>

christianvogt · 2020-07-07T17:39:01Z

frontend/packages/dev-console/src/components/GuidedTourTile.tsx

+const GuidedTourTile: React.FC<GuidedTourTileProps> = ({ tours }) => {
+  const [isOpen, setOpen] = React.useState(false);
+  const onToggle = () => setOpen(!isOpen);
+  const actionDropdownItem = [<DropdownItem key="link">Remove guided tours</DropdownItem>];


This action needs to remove the card and save the setting to local storage.

christianvogt · 2020-07-07T17:40:46Z

frontend/packages/dev-console/src/components/GuidedTourTile.tsx

+              isOpen={isOpen}
+              isPlain
+              dropdownItems={actionDropdownItem}
+              position={'right'}


Suggested change

position={'right'}

position="right"

christianvogt · 2020-07-08T21:28:39Z

frontend/packages/dev-console/src/utils/guided-tours/guided-tour-action.ts

@@ -0,0 +1,18 @@
+import { action, ActionType } from 'typesafe-actions';
+
+export const TOUR_TILE_STORAGE_KEY = 'bridge/guided_tour_tile';


Suggested change

export const TOUR_TILE_STORAGE_KEY = 'bridge/guided_tour_tile';

export const TOUR_TILE_STORAGE_KEY = 'bridge/hide-guided_tour_tile';

christianvogt · 2020-07-08T21:34:37Z

frontend/packages/dev-console/src/components/GuidedTourTile.tsx

+  tours: TourItem[];
+};
+
+const GuidedTourTile: React.FC<GuidedTourTileProps & DispatchProps> = ({ tours, removeTours }) => {


Using redux for this seems like overkill when there's only 1 spot in the app where we need this functionality.
Wouldn't something simple like this do the trick:

const [showTile, setShowTile] = React.useState(!localStorage.get(HIDE_TOUR_TILE_STORAGE_KEY)); const onRemove = () => { localStorage.set(HIDE_TOUR_TILE_STORAGE_KEY, true); setShowTile(false); }; ... return showTile ? ... : null;

@rohitkrai03 thoughts?

I agree with @christianvogt here, it doesn't make sense to use redux if the state is only ever going to be used in this component.

christianvogt · 2020-07-08T21:35:00Z

frontend/packages/dev-console/src/components/GuidedTourTile.scss

+  &__header {
+    padding: 0px var(--pf-global--spacer--md);
+    margin: 0px;
+  }


Remove as this is no longer used.

rohitkrai03 · 2020-07-09T12:56:00Z

frontend/packages/dev-console/src/components/GuidedTourTile.tsx

+type DispatchProps = {
+  removeTours?: () => void;
+};


You don't need this.

rohitkrai03 · 2020-07-09T12:56:24Z

frontend/packages/dev-console/src/components/GuidedTourTile.tsx

+
+const HIDE_TOUR_TILE_STORAGE_KEY = 'bridge/hide-tour-tile';
+const GuidedTourTile: React.FC<GuidedTourTileProps & DispatchProps> = ({ tours }) => {
+  const isTourTileHidden = !!localStorage.getItem(HIDE_TOUR_TILE_STORAGE_KEY) || false;


Suggested change

const isTourTileHidden = !!localStorage.getItem(HIDE_TOUR_TILE_STORAGE_KEY) || false;

const isTourTileHidden = !!localStorage.getItem(HIDE_TOUR_TILE_STORAGE_KEY) ?? false;

rohitkrai03 · 2020-07-09T12:58:11Z

frontend/packages/dev-console/src/components/GuidedTourTile.tsx

+    </DropdownItem>,
+  ];
+
+  const orderedTours = tours.length > 3 ? tours.slice(0, 3) : tours;


It's not actually ordered tours.

Suggested change

const orderedTours = tours.length > 3 ? tours.slice(0, 3) : tours;

const slicedTours = tours.length > 3 ? tours.slice(0, 3) : tours;

rohitkrai03 · 2020-07-09T12:59:13Z

frontend/packages/dev-console/src/components/__tests__/GuidedTourTile.spec.tsx

+    tours: getGuidedToursWithStatus(),
+  };
+  const guidedTourTileWrapper = shallow(<GuidedTourTile {...guidedTourTileProps} />);
+  it('should show proper CardAction', () => {


Would be good to have a test for when local storage has removed tile state.

rohitkrai03 · 2020-07-09T13:33:59Z

frontend/packages/dev-console/src/components/GuidedTourAddAction.tsx

+import { getGuidedToursWithStatus } from '@console/app/src/components/guided-tours/utils/guided-tour-utils';
+import GuidedTourTile from './GuidedTourTile';
+
+const GuidedTourAddAction: React.FC = () => {


Why exactly do we need this extra component here? It doesn't seem to be doing anything at the moment.

I've removed this

rohitkrai03 · 2020-07-09T15:50:32Z

frontend/packages/dev-console/src/components/EmptyState.tsx

@@ -71,6 +74,9 @@ const ODCEmptyState: React.FC<Props> = ({
  const addActionExtensions = useExtensions<AddAction>(
    isAddAction,
  ).filter(({ properties: { hide } }) => (hide ? hide() : true));
+
+  const guidedTourList = getGuidedToursWithStatus();


Fetching of tour could easily be handled by GuidedTourTile component itself. If there are tours it will render the tile else just return null.

I didn't care much about where the mock data is coming from but I agree with Rohit here that it makes more sense to have this in GuidedTourTile.

christianvogt · 2020-07-09T18:07:30Z

frontend/packages/dev-console/src/components/GuidedTourTile.scss

@@ -0,0 +1,16 @@
+.odc-guidedtour-tile {
+  &__card {
+    height: 100%;


height: $co-m-catalog-tile-height; width: $co-m-catalog-tile-width;

The above aligns with the other empty state tile sizes.

christianvogt · 2020-07-09T18:07:54Z

frontend/packages/dev-console/src/components/EmptyState.tsx

@@ -71,6 +74,9 @@ const ODCEmptyState: React.FC<Props> = ({
  const addActionExtensions = useExtensions<AddAction>(
    isAddAction,
  ).filter(({ properties: { hide } }) => (hide ? hide() : true));
+
+  const guidedTourList = getGuidedToursWithStatus();


I didn't care much about where the mock data is coming from but I agree with Rohit here that it makes more sense to have this in GuidedTourTile.

rohitkrai03

/lgtm

openshift-ci-robot · 2020-07-09T19:43:18Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinandan13jan, rohitkrai03

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~frontend/packages/dev-console/OWNERS~~ [rohitkrai03]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci-robot added the component/core Related to console core functionality label Jun 25, 2020

openshift-ci-robot requested review from rottencandy and spadgett June 25, 2020 11:25

openshift-ci-robot added the component/dev-console Related to dev-console label Jun 25, 2020

abhinandan13jan changed the title ~~Guided tour tile~~ [WIP] -- add Guided tour tile Jun 25, 2020

openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 25, 2020

abhinandan13jan changed the title ~~[WIP] -- add Guided tour tile~~ add Guided tour tile Jun 30, 2020

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 30, 2020

christianvogt reviewed Jun 30, 2020

View reviewed changes

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 1, 2020

abhinandan13jan force-pushed the guided-tour-tile branch from e173091 to a21046a Compare July 2, 2020 08:18

openshift-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jul 2, 2020

add guidedTourAction in Add page

94765cc

abhinandan13jan force-pushed the guided-tour-tile branch from a8e4fa0 to 94765cc Compare July 7, 2020 13:10

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 7, 2020

christianvogt reviewed Jul 7, 2020

View reviewed changes

christianvogt reviewed Jul 8, 2020

View reviewed changes

abhinandan13jan force-pushed the guided-tour-tile branch from 7cedaaf to ba80d2d Compare July 9, 2020 11:07

rohitkrai03 suggested changes Jul 9, 2020

View reviewed changes

abhinandan13jan force-pushed the guided-tour-tile branch from ba80d2d to ebdb877 Compare July 9, 2020 13:25

rohitkrai03 suggested changes Jul 9, 2020

View reviewed changes

abhinandan13jan force-pushed the guided-tour-tile branch from ebdb877 to 025efc5 Compare July 9, 2020 15:41

rohitkrai03 suggested changes Jul 9, 2020

View reviewed changes

christianvogt reviewed Jul 9, 2020

View reviewed changes

abhinandan13jan force-pushed the guided-tour-tile branch from 025efc5 to 2be8038 Compare July 9, 2020 18:46

add redux localstore

76a53de

abhinandan13jan force-pushed the guided-tour-tile branch from 2be8038 to 76a53de Compare July 9, 2020 19:02

rohitkrai03 approved these changes Jul 9, 2020

View reviewed changes

openshift-ci-robot assigned rohitkrai03 Jul 9, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 9, 2020

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 9, 2020

openshift-merge-robot merged commit f472a52 into openshift:master Jul 9, 2020

spadgett added this to the v4.6 milestone Jul 14, 2020

	padding: var(--pf-global--spacer--sm) var(--pf-global--spacer--lg);
	padding-top: var(--pf-global--spacer--sm);
	padding-bottom: var(--pf-global--spacer--sm);

	margin-right: var(--pf-global--spacer--md);
	margin-right: var(--pf-global--spacer--sm);

		padding: var(--pf-global--spacer--sm) var(--pf-global--spacer--md);
		text-align: left;

	padding: var(--pf-global--spacer--sm) var(--pf-global--spacer--md);
	text-align: left;
	margin-bottom: var(--pf-global--spacer--sm);

		@@ -0,0 +1,18 @@
		import { action, ActionType } from 'typesafe-actions';

		export const TOUR_TILE_STORAGE_KEY = 'bridge/guided_tour_tile';

	const isTourTileHidden = !!localStorage.getItem(HIDE_TOUR_TILE_STORAGE_KEY) \|\| false;
	const isTourTileHidden = !!localStorage.getItem(HIDE_TOUR_TILE_STORAGE_KEY) ?? false;

	const orderedTours = tours.length > 3 ? tours.slice(0, 3) : tours;
	const slicedTours = tours.length > 3 ? tours.slice(0, 3) : tours;

add Guided tour tile #5822

add Guided tour tile #5822

Conversation

abhinandan13jan commented Jun 25, 2020 • edited

Adresses

Issue

Solution

ScreenShot

Tests

Browser Conformation

christianvogt commented Jun 25, 2020

abhinandan13jan commented Jun 30, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

christianvogt commented Jul 6, 2020

abhinandan13jan commented Jul 7, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abhinandan13jan Jul 9, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rohitkrai03 left a comment

Choose a reason for hiding this comment

openshift-ci-robot commented Jul 9, 2020

abhinandan13jan commented Jun 25, 2020 •

edited

abhinandan13jan Jul 9, 2020 •

edited