Nv 1787 notifications page placeholder #2954

LetItRock · 2023-03-06T13:34:11Z

What change does this PR introduce?

The Notifications page table placeholder will be shown when there are no templates created yet.
The "Try Digest Playground" tile creates the example "Digest Workflow" and currently it redirects to the Workflow Editor but will navigate to the Digest Workflow Playground later on.

Why was this change needed?

Other information (Screenshots)

…tifications-page-placeholder

linear · 2023-03-06T13:34:14Z

NV-1787 Implement the Notifications page Empty State

When the user doesn't have any workflows setup yet or he has removed all the workflows we want in that case to show the placeholder with two buttons to create a new workflow or to try the digest playground.

Screenshot 2023-03-03 at 11.33.22.png

LetItRock · 2023-03-06T13:34:29Z

apps/web/.storybook/preview-head.html

+<script>
+window._env_ = {
+  SKIP_PREFLIGHT_CHECK: 'true',
+};
+</script>


fix for the storybook, it was failing because this env did not exist

LetItRock · 2023-03-06T13:36:02Z

apps/web/cypress/plugins/index.ts

@@ -85,7 +89,7 @@ module.exports = (on, config) => {
      );

      let templates;
-      if (!settings?.noEnvironment) {
+      if (!settings?.noTemplates) {


the noEnvironment was not used...
if noTemplates flag is set we won't create the templates in Cypress e2e tests...

LetItRock · 2023-03-06T13:37:18Z

apps/web/cypress/tests/notifications.spec.ts

+  it('when no workflow templates created it should show the page placeholder', function () {
+    cy.initializeSession({ noTemplates: true }).as('session');
+    cy.intercept('**/notification-templates**').as('notificationTemplates');
+    cy.visit('/templates');
+    cy.wait('@notificationTemplates');
+
+    cy.getByTestId('no-workflow-templates-placeholder').should('be.visible');
+    cy.getByTestId('create-workflow-tile').should('not.be.disabled');
+    cy.getByTestId('try-digest-playground-tile').should('not.be.disabled');
+  });
+
+  it('when clicking on create workflow it should redirect to create template page', function () {
+    cy.initializeSession({ noTemplates: true }).as('session');
+    cy.intercept('**/notification-templates**').as('notificationTemplates');
+    cy.visit('/templates');
+    cy.wait('@notificationTemplates');
+
+    cy.getByTestId('no-workflow-templates-placeholder').should('be.visible');
+    cy.getByTestId('create-workflow-tile').should('not.be.disabled');
+    cy.getByTestId('create-workflow-tile').click();
+
+    cy.url().should('include', '/templates/create');
+  });


for now only two tests...
I'll add the test for the Try Digest Workflow tile a little later when it will point to the appropriate page

LetItRock · 2023-03-06T13:38:08Z

apps/web/src/design-system/cards/Cards.tsx

  imagePath?: string;
  href?: string;
+  disabled?: boolean;
+  testId?: string;
 }

 export function Cards({ cells }: { cells: ICardCell[] }) {


moved this component to the design-system folder and adjusted it a little to be more close with the designs

LetItRock · 2023-03-06T13:38:53Z

apps/web/src/design-system/icons/general/DigestGradient.tsx

-}
-/* eslint-disable */
-export function DigestGradient({ width = '30', height = '31' }: IIconProps) {
+export function DigestGradient(props: React.ComponentPropsWithoutRef<'svg'>) {


allow to override the width and height like we do in other icons

Wasn't working before?

it was only allowing for the width/height, but not for ex. styles

LetItRock · 2023-03-06T13:40:50Z

apps/web/src/pages/quick-start/steps/Quickstart.tsx

@@ -39,6 +38,8 @@ export function Quickstart() {
  );
 }

+const BellGradientIcon = () => <BellGradient style={{ width: '40px', height: '40px' }} />;


our icons are looking wrong when you do inspect its viewBox... that's why I had to do this hack in order to have them look the same... I'll talk to Nikolay about it

LetItRock · 2023-03-06T13:41:26Z

apps/web/src/design-system/table/Table.tsx

@@ -25,6 +45,7 @@ export function Table({
  pagination = false,
  loading = false,
  onRowClick,
+  noDataPlaceholder,


allow to pass the table placeholder

LetItRock · 2023-03-06T13:42:24Z

apps/web/src/design-system/table/Table.tsx

@@ -79,7 +100,7 @@ export function Table({
        : {}),
    } as any,
    usePagination
-  );
+  ) as UseTableProps;


adjusted types for the useTable hook... casted it here to the UseTableProps because there is no another way to do this when the hook uses other plugins, like usePagination

LetItRock · 2023-03-06T13:43:50Z

apps/web/src/pages/templates/TemplatesListPage.tsx

@@ -31,6 +51,39 @@ function NotificationList() {
    navigate(ROUTES.TEMPLATES_CREATE);
  };

+  const handleTryDigestClick = () => {


when the Try Digest Workflow button is clicked we do create an example Digest Workflow

Not sure if this should be driven (the example workflow content) by the backend. 🤔

we do the same for the in-app workflow and I think that for now, it's ok to have it like this...
I feel like we should use what is available to accomplish the goal and not overengineer with the FE needs...

p-fernandez · 2023-03-06T13:51:27Z

apps/web/src/design-system/cards/Cards.tsx

-    <StyledCard dark={colorScheme === 'dark'} onClick={handleOnClick}>
+    <StyledCard
+      dark={colorScheme === 'dark'}
+      onClick={cell.onClick ?? handleOnClick}


I think this logic should be moved to inside handleOnClick.

Suggested change

onClick={cell.onClick ?? handleOnClick}

onClick={handleOnClick}

And inside of the method:

const handleOnClick = () => { if (cell.onClick) { onClick(); return; } if (cell.navigateTo) { navigate(cell.navigateTo); } };

p-fernandez · 2023-03-06T13:52:54Z

apps/web/src/design-system/icons/general/DigestGradient.tsx

-}
-/* eslint-disable */
-export function DigestGradient({ width = '30', height = '31' }: IIconProps) {
+export function DigestGradient(props: React.ComponentPropsWithoutRef<'svg'>) {


Wasn't working before?

p-fernandez · 2023-03-06T13:57:22Z

apps/web/src/pages/templates/TemplatesListNoData.tsx

+  );
+};
+
+const DigestGradientIcon = () => <DigestGradient style={{ height: '40px', width: '40px' }} />;


Shouldn't this live as a reusable component within the new FE structure?

the DigestGradient is imported from the design system and is supposed to be a reusable icon, but its viewport is broken so the icon is not looking good everywhere, that's why I've hacked it like this to look similar in size to the other icon... talked about this with Nikolay and we plan to update the icons soon

p-fernandez · 2023-03-06T13:58:59Z

apps/web/src/pages/templates/TemplatesListPage.tsx

@@ -31,6 +51,39 @@ function NotificationList() {
    navigate(ROUTES.TEMPLATES_CREATE);
  };

+  const handleTryDigestClick = () => {


Not sure if this should be driven (the example workflow content) by the backend. 🤔

LetItRock added 3 commits March 6, 2023 14:25

chore(web): notifications page table placeholder

111813f

Merge branch 'next' into nv-1787-notifications-page-placeholder

013a71e

Merge branch 'nv-1786-refactor-workflow-editor-state' into nv-1787-no…

a449354

…tifications-page-placeholder

LetItRock self-assigned this Mar 6, 2023

github-actions bot added the @novu/web label Mar 6, 2023

Format code with prettier

3409886

LetItRock commented Mar 6, 2023

View reviewed changes

p-fernandez approved these changes Mar 6, 2023

View reviewed changes

Base automatically changed from nv-1786-refactor-workflow-editor-state to getting-started-flow March 6, 2023 15:22

chore(web): add suggestions from the pr #2954

ab86fbe

LetItRock merged commit d6a555a into getting-started-flow Mar 6, 2023

LetItRock deleted the nv-1787-notifications-page-placeholder branch March 6, 2023 17:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Nv 1787 notifications page placeholder #2954

Nv 1787 notifications page placeholder #2954

LetItRock commented Mar 6, 2023

linear bot commented Mar 6, 2023

LetItRock Mar 6, 2023

LetItRock Mar 6, 2023

LetItRock Mar 6, 2023

LetItRock Mar 6, 2023

LetItRock Mar 6, 2023

p-fernandez Mar 6, 2023

LetItRock Mar 6, 2023

LetItRock Mar 6, 2023

LetItRock Mar 6, 2023

LetItRock Mar 6, 2023

LetItRock Mar 6, 2023

p-fernandez Mar 6, 2023

LetItRock Mar 6, 2023

p-fernandez Mar 6, 2023

p-fernandez Mar 6, 2023

p-fernandez Mar 6, 2023

LetItRock Mar 6, 2023

p-fernandez Mar 6, 2023

	onClick={cell.onClick ?? handleOnClick}
	onClick={handleOnClick}

Nv 1787 notifications page placeholder #2954

Nv 1787 notifications page placeholder #2954

Conversation

LetItRock commented Mar 6, 2023

What change does this PR introduce?

Why was this change needed?

Other information (Screenshots)

linear bot commented Mar 6, 2023

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