From 2376bcafb486c2a1f309448372ca0d75c5c19b7a Mon Sep 17 00:00:00 2001 From: Jeff Daley Date: Tue, 17 Oct 2023 20:01:03 -0400 Subject: [PATCH] Update "Choose a template" screen (#366) * Update and test doc template screen * Add helper test * Change icon color to gray --- .../components/new/document-template-list.hbs | 91 +++++++++++++ .../components/new/document-template-list.ts | 20 +++ web/app/helpers/get-doctype-icon.ts | 38 ++++++ web/app/styles/app.scss | 1 - web/app/styles/components/template-card.scss | 32 ----- web/app/styles/typography.scss | 4 + web/app/templates/authenticated/new/index.hbs | 47 +------ web/app/types/document-type.d.ts | 1 + web/mirage/config.ts | 68 ++++------ web/mirage/factories/document-type.ts | 7 + web/mirage/models/document-type.ts | 5 + .../acceptance/authenticated/new-test.ts | 120 +++++++++++++++++- .../helpers/get-doctype-icon-test.ts | 31 +++++ 13 files changed, 343 insertions(+), 122 deletions(-) create mode 100644 web/app/components/new/document-template-list.hbs create mode 100644 web/app/components/new/document-template-list.ts create mode 100644 web/app/helpers/get-doctype-icon.ts delete mode 100644 web/app/styles/components/template-card.scss create mode 100644 web/mirage/factories/document-type.ts create mode 100644 web/mirage/models/document-type.ts create mode 100644 web/tests/integration/helpers/get-doctype-icon-test.ts diff --git a/web/app/components/new/document-template-list.hbs b/web/app/components/new/document-template-list.hbs new file mode 100644 index 000000000..2e537c307 --- /dev/null +++ b/web/app/components/new/document-template-list.hbs @@ -0,0 +1,91 @@ +
+

Choose a template

+ {{! PENDING }} + {{! +
+ or +
+ + }} +
+ +
    + {{#each @docTypes as |docType|}} +
  1. + + +
    + +
    +
    +

    + {{docType.longName}} +

    + {{#unless + (eq (lowercase docType.longName) (lowercase docType.name)) + }} +

    + {{docType.name}} +

    + {{/unless}} +
    +

    + {{docType.description}} +

    +
    +
    +     +     +
    2. + {{/each}} +
+ +{{#if this.moreInfoLinksAreShown}} +
+

+ Related links +

+ +
+{{/if}} diff --git a/web/app/components/new/document-template-list.ts b/web/app/components/new/document-template-list.ts new file mode 100644 index 000000000..406603e6b --- /dev/null +++ b/web/app/components/new/document-template-list.ts @@ -0,0 +1,20 @@ +import Component from "@glimmer/component"; +import { HermesDocumentType } from "hermes/types/document-type"; + +interface NewDocumentTemplateListComponentSignature { + Args: { + docTypes: HermesDocumentType[]; + }; +} + +export default class NewDocumentTemplateListComponent extends Component { + protected get moreInfoLinksAreShown(): boolean { + return this.args.docTypes.some((docType) => docType.moreInfoLink); + } +} + +declare module "@glint/environment-ember-loose/registry" { + export default interface Registry { + "New::DocumentTemplateList": typeof NewDocumentTemplateListComponent; + } +} diff --git a/web/app/helpers/get-doctype-icon.ts b/web/app/helpers/get-doctype-icon.ts new file mode 100644 index 000000000..bfa947b83 --- /dev/null +++ b/web/app/helpers/get-doctype-icon.ts @@ -0,0 +1,38 @@ +import { helper } from "@ember/component/helper"; + +export interface GetDoctypeIconSignature { + Args: { + Positional: [string]; + }; + Return: string; +} + +/** + * Returns the FlightIcon name for a given document type. + * This is a temporary helper until "flight_icon" is added + * to the HermesDocumentType model. + */ +const getDoctypeIconHelper = helper( + ([docTypeName]) => { + switch (docTypeName.toLowerCase()) { + case "rfc": + return "discussion-circle"; + case "prd": + return "target"; + case "frd": + return "dollar-sign"; + case "memo": + return "radio"; + default: + return "file-text"; + } + }, +); + +export default getDoctypeIconHelper; + +declare module "@glint/environment-ember-loose/registry" { + export default interface Registry { + "get-doctype-icon": typeof getDoctypeIconHelper; + } +} diff --git a/web/app/styles/app.scss b/web/app/styles/app.scss index 79aa2b4a0..17a98355c 100644 --- a/web/app/styles/app.scss +++ b/web/app/styles/app.scss @@ -22,7 +22,6 @@ @use "components/multiselect"; @use "components/page"; @use "components/row-results"; -@use "components/template-card"; @use "components/header/active-filter-list"; @use "components/header/active-filter-list-item"; @use "components/header/search"; diff --git a/web/app/styles/components/template-card.scss b/web/app/styles/components/template-card.scss deleted file mode 100644 index 6d0440472..000000000 --- a/web/app/styles/components/template-card.scss +++ /dev/null @@ -1,32 +0,0 @@ -.template-card { - @apply p-5 h-full flex flex-col justify-between duration-300 cursor-pointer; - - transition-property: background, box-shadow; - background: linear-gradient( - to bottom, - var(--token-color-surface-interactive), - var(--token-color-surface-interactive) - ); - - &:hover { - background: linear-gradient( - to bottom, - var(--token-color-surface-interactive), - var(--token-color-surface-action) - ); - } - - &.disabled { - @apply cursor-default transition-none; - - background: var(--token-color-surface-faint); - - &:hover { - background: var(--token-color-surface-faint); - } - } - - &--with-link { - @apply pb-12; - } -} diff --git a/web/app/styles/typography.scss b/web/app/styles/typography.scss index 9239c057d..0172b71c1 100644 --- a/web/app/styles/typography.scss +++ b/web/app/styles/typography.scss @@ -12,6 +12,10 @@ @apply text-body-100; } +.hermes-h-300 { + @apply text-display-300 font-semibold text-color-foreground-strong; +} + .hermes-h4 { @apply text-body-100 font-medium uppercase tracking-wide text-color-foreground-faint; } diff --git a/web/app/templates/authenticated/new/index.hbs b/web/app/templates/authenticated/new/index.hbs index 7bea2e7ec..5dcdc6584 100644 --- a/web/app/templates/authenticated/new/index.hbs +++ b/web/app/templates/authenticated/new/index.hbs @@ -1,46 +1,3 @@ -{{page-title "New Doc"}} +{{page-title "Choose a template"}} -

Choose a template

-
    - {{#each @model as |docType|}} -
  1. - - -
    -

    - {{docType.name}} -

    -

    - {{docType.longName}} -

    -

    - {{docType.description}} -

    -
    -     -     - {{#if docType.moreInfoLink}} - - {{/if}} -
    2. - {{/each}} -
+ diff --git a/web/app/types/document-type.d.ts b/web/app/types/document-type.d.ts index 66820978c..0ce222c11 100644 --- a/web/app/types/document-type.d.ts +++ b/web/app/types/document-type.d.ts @@ -14,6 +14,7 @@ export interface HermesDocumentType { name: string; longName: string; description: string; + flightIcon?: string; moreInfoLink?: { text: string; url: string; diff --git a/web/mirage/config.ts b/web/mirage/config.ts index 5206330f8..4cd5b7491 100644 --- a/web/mirage/config.ts +++ b/web/mirage/config.ts @@ -306,49 +306,33 @@ export default function (mirageConfig) { * Used in the /new routes when creating a document. */ this.get("/document-types", () => { - return new Response(200, {}, [ - { - name: "RFC", - longName: "Request for Comments", - description: - "Create a Request for Comments document to present a proposal to colleagues for their review and feedback.", - moreInfoLink: { - text: "More-info link", - url: "example.com", - }, - checks: [ - { - label: "I have read the Terms and Conditions", - helperText: - "Please read the Terms and Conditions before proceeding.", - links: [ - { - text: "Terms and Conditions", - url: "example.com", - }, - ], - }, - ], - customFields: [ - { - name: "Current Version", - readOnly: false, - type: "string", - }, - { - name: "Stakeholders", - readOnly: false, - type: "people", + if (this.schema.documentTypes.all().models.length === 0) { + return new Response(200, {}, [ + { + name: "RFC", + longName: "Request for Comments", + description: + "Present a proposal to colleagues for their review and feedback.", + moreInfoLink: { + text: "More-info link", + url: "example.com", }, - ], - }, - { - name: "PRD", - longName: "Product Requirements", - description: - "Create a Product Requirements Document to summarize a problem statement and outline a phased approach to addressing the problem.", - }, - ]); + }, + { + name: "PRD", + longName: "Product Requirements", + description: + "Summarize a problem statement and outline a phased approach to addressing it.", + }, + ]); + } + return new Response( + 200, + {}, + this.schema.documentTypes.all().models.map((docType) => { + return docType.attrs; + }), + ); }); /** diff --git a/web/mirage/factories/document-type.ts b/web/mirage/factories/document-type.ts new file mode 100644 index 000000000..909bb349e --- /dev/null +++ b/web/mirage/factories/document-type.ts @@ -0,0 +1,7 @@ +import { Factory } from "miragejs"; + +export default Factory.extend({ + name: (i: number) => `DT${i}`, + longName: (i: number) => `Document Type ${i}`, + description: "This is a test document type", +}); diff --git a/web/mirage/models/document-type.ts b/web/mirage/models/document-type.ts new file mode 100644 index 000000000..dea7fc045 --- /dev/null +++ b/web/mirage/models/document-type.ts @@ -0,0 +1,5 @@ +import { Model } from "miragejs"; + +export default Model.extend({ + // Required for Mirage, even though it's empty +}); diff --git a/web/tests/acceptance/authenticated/new-test.ts b/web/tests/acceptance/authenticated/new-test.ts index c55839129..5c8c1c20d 100644 --- a/web/tests/acceptance/authenticated/new-test.ts +++ b/web/tests/acceptance/authenticated/new-test.ts @@ -5,19 +5,135 @@ import { authenticateSession } from "ember-simple-auth/test-support"; import { MirageTestContext, setupMirage } from "ember-cli-mirage/test-support"; import { getPageTitle } from "ember-page-title/test-support"; +const TEMPLATE_OPTION = "[data-test-template-option]"; +const ICON = `${TEMPLATE_OPTION} [data-test-icon]`; +const LONG_NAME = `${TEMPLATE_OPTION} [data-test-long-name]`; +const NAME = `${TEMPLATE_OPTION} [data-test-name]`; +const DESCRIPTION = `${TEMPLATE_OPTION} [data-test-description]`; +const MORE_INFO_LINK = `[data-test-more-info-link]`; + interface AuthenticatedNewRouteTestContext extends MirageTestContext {} module("Acceptance | authenticated/new", function (hooks) { setupApplicationTest(hooks); setupMirage(hooks); - hooks.beforeEach(async function () { + hooks.beforeEach(async function (this: AuthenticatedNewRouteTestContext) { await authenticateSession({}); + + this.server.create("document-type", { + name: "FD", + longName: "Foo Document", + description: "Foo", + moreInfoLink: { + text: "Foo", + url: "https://example.com/foo", + }, + }); + + this.server.create("document-type", { + name: "RFC", + longName: "Request for Comments", + description: "RFC", + flightIcon: "square", + }); + + this.server.create("document-type", { + name: "MEMO", + longName: "Memo", + description: "Memo", + moreInfoLink: { + text: "Memo", + url: "https://example.com/memo", + }, + }); }); test("the page title is correct", async function (this: AuthenticatedNewRouteTestContext, assert) { await visit("/new"); await waitFor("h1"); - assert.equal(getPageTitle(), "New Doc | Hermes"); + assert.equal(getPageTitle(), "Choose a template | Hermes"); + }); + + test("it renders as expected", async function (this: AuthenticatedNewRouteTestContext, assert) { + await visit("/new"); + + assert.dom(TEMPLATE_OPTION).exists({ count: 3 }); + + const expectedLongNames = ["Foo Document", "Request for Comments", "Memo"]; + + assert.deepEqual( + [...document.querySelectorAll(LONG_NAME)].map( + (el) => el.textContent?.trim(), + ), + expectedLongNames, + ); + + const expectedDescriptions = ["Foo", "RFC", "Memo"]; + + assert.deepEqual( + [...document.querySelectorAll(DESCRIPTION)].map( + (el) => el.textContent?.trim(), + ), + expectedDescriptions, + ); + + // We don't expect "MEMO" because it's basically the same as "Memo" + + const expectedNames = ["FD", "RFC"]; + + assert.deepEqual( + [...document.querySelectorAll(NAME)].map((el) => el.textContent?.trim()), + expectedNames, + ); + + // `file-text` is the default icon + // `square` is the icon we just set for RFC. + // `radio` is what's returned from `getDoctypeIcon` for "Memo" + + const expectedIcons = ["file-text", "square", "radio"]; + + assert.deepEqual( + [...document.querySelectorAll(ICON)].map((el) => + el.getAttribute("data-test-icon"), + ), + expectedIcons, + ); + + assert.dom(MORE_INFO_LINK).exists({ count: 2 }); + + const expectedLinkText = ["Foo", "Memo"]; + const expectedLinkHREFs = [ + "https://example.com/foo", + "https://example.com/memo", + ]; + + assert.deepEqual( + [...document.querySelectorAll(MORE_INFO_LINK)].map((el) => + el.getAttribute("href"), + ), + expectedLinkHREFs, + ); + + assert.deepEqual( + [...document.querySelectorAll(MORE_INFO_LINK)].map( + (el) => el.textContent?.trim(), + ), + expectedLinkText, + ); + }); + + test(`it doesn't render the "more info" links if they're not present`, async function (this: AuthenticatedNewRouteTestContext, assert) { + this.server.db.emptyData(); + + this.server.create("document-type", { + name: "BD", + longName: "Bar Document", + description: "Bar", + }); + + await visit("/new"); + + assert.dom(MORE_INFO_LINK).doesNotExist(); }); }); diff --git a/web/tests/integration/helpers/get-doctype-icon-test.ts b/web/tests/integration/helpers/get-doctype-icon-test.ts new file mode 100644 index 000000000..4b614768a --- /dev/null +++ b/web/tests/integration/helpers/get-doctype-icon-test.ts @@ -0,0 +1,31 @@ +import { module, test } from "qunit"; +import { setupRenderingTest } from "ember-qunit"; +import { render } from "@ember/test-helpers"; +import { hbs } from "ember-cli-htmlbars"; + +module("Integration | Helper | get-doctype-icon", function (hooks) { + setupRenderingTest(hooks); + + test("it returns the expected values", async function (assert) { + await render(hbs` + {{get-doctype-icon "RFC"}} + {{get-doctype-icon "PRD"}} + {{get-doctype-icon "FRD"}} + {{get-doctype-icon "MEMO"}} + {{get-doctype-icon "FOO"}} + `); + + const expectedValues = [ + "discussion-circle", + "target", + "dollar-sign", + "radio", + "file-text", + ]; + + assert.deepEqual( + this.element.textContent?.trim().split(/\s+/), + expectedValues, + ); + }); +});