From 026e7a878c3dd137c4b5522de2959e853316cc2f Mon Sep 17 00:00:00 2001 From: Seyi Adebajo Date: Tue, 25 Sep 2018 10:22:19 -0700 Subject: [PATCH 1/3] refactors implementation and design of stacked-avatars-list component: reuses hover-dropdown component to list menu options. repositions component in entity header. adds new core type definition file. implements function to construct a mail-to uri. --- .../avatars/stacked-avatars-list.ts | 61 ++++++++++++------- .../datasets/containers/dataset-owner-list.ts | 12 ++-- wherehows-web/app/constants/datasets/owner.ts | 26 +++++++- wherehows-web/app/styles/components/_all.scss | 1 + .../app/styles/components/avatar/_avatar.scss | 30 ++++++++- .../styles/components/dataset-owner/_all.scss | 1 + .../components/dataset-owner/_owners.scss | 5 ++ wherehows-web/app/styles/layout/_dataset.scss | 8 --- .../avatars/stacked-avatars-list.hbs | 30 +++++---- .../nacho/dropdown/hover-dropdown.hbs | 36 ++++++----- .../app/templates/datasets/dataset.hbs | 8 +-- wherehows-web/app/typings/app/avatars.d.ts | 14 ++++- wherehows-web/app/typings/app/core.d.ts | 14 +++++ .../app/typings/app/helpers/email.d.ts | 18 ++++++ wherehows-web/app/utils/helpers/email.ts | 22 +++++++ .../validators/email/email-test-fixture.ts | 59 ++++++++++++++++++ .../tests/unit/utils/helpers/email-test.ts | 17 ++++++ 17 files changed, 287 insertions(+), 75 deletions(-) create mode 100644 wherehows-web/app/styles/components/dataset-owner/_all.scss create mode 100644 wherehows-web/app/styles/components/dataset-owner/_owners.scss create mode 100644 wherehows-web/app/typings/app/core.d.ts create mode 100644 wherehows-web/app/typings/app/helpers/email.d.ts create mode 100644 wherehows-web/app/utils/helpers/email.ts create mode 100644 wherehows-web/tests/helpers/validators/email/email-test-fixture.ts create mode 100644 wherehows-web/tests/unit/utils/helpers/email-test.ts diff --git a/wherehows-web/app/components/avatars/stacked-avatars-list.ts b/wherehows-web/app/components/avatars/stacked-avatars-list.ts index 0e18663f39..a13cf26b96 100644 --- a/wherehows-web/app/components/avatars/stacked-avatars-list.ts +++ b/wherehows-web/app/components/avatars/stacked-avatars-list.ts @@ -1,7 +1,8 @@ import Component from '@ember/component'; -import { get, getProperties, computed } from '@ember/object'; -import ComputedProperty from '@ember/object/computed'; -import { IAvatar } from 'wherehows-web/typings/app/avatars'; +import { IAvatar, IAvatarDropDownAction } from 'wherehows-web/typings/app/avatars'; +import { action, computed } from '@ember-decorators/object'; +import { IDropDownOption } from 'wherehows-web/typings/app/dataset-compliance'; +import { classNames } from '@ember-decorators/component'; /** * Specifies the default maximum number of images to render before the more button @@ -9,8 +10,13 @@ import { IAvatar } from 'wherehows-web/typings/app/avatars'; */ const defaultMavAvatarLength = 6; +@classNames('avatar-container') export default class StackedAvatarsList extends Component { - classNames = ['avatar-container']; + /** + * The list of avatar objects to render + * @type {Array} + */ + avatars: Array; constructor() { super(...arguments); @@ -18,44 +24,53 @@ export default class StackedAvatarsList extends Component { this.avatars || (this.avatars = []); } - /** - * The list of avatar objects to render - * @type {Array} - */ - avatars: Array; - /** * Calculates the max number of avatars to render * @type {ComputedProperty} * @memberof StackedAvatarsList */ - maxAvatarLength: ComputedProperty = computed('avatars.length', function(this: StackedAvatarsList): number { - const { length } = get(this, 'avatars'); + @computed('avatars.length') + get maxAvatarLength(): number { + const { + avatars: { length } + } = this; return length ? Math.min(length, defaultMavAvatarLength) : defaultMavAvatarLength; - }); + } /** * Build the list of avatars to render based on the max number * @type {ComputedProperty} * @memberof StackedAvatarsList */ - maxAvatars: ComputedProperty = computed('maxAvatarLength', function( - this: StackedAvatarsList - ): StackedAvatarsList['avatars'] { - const { avatars, maxAvatarLength } = getProperties(this, ['avatars', 'maxAvatarLength']); + @computed('maxAvatarLength') + get maxAvatars(): StackedAvatarsList['avatars'] { + const { avatars, maxAvatarLength } = this; return avatars.slice(0, maxAvatarLength); - }); + } /** * Determines the list of avatars that have not been rendered after the max has been ascertained * @type {ComputedProperty} * @memberof StackedAvatarsList */ - rollupAvatars: ComputedProperty = computed('maxAvatars', function( - this: StackedAvatarsList - ): StackedAvatarsList['avatars'] { - const { avatars, maxAvatarLength } = getProperties(this, ['avatars', 'maxAvatarLength']); + @computed('maxAvatars') + get rollupAvatars(): StackedAvatarsList['avatars'] { + const { avatars, maxAvatarLength } = this; + return avatars.slice(maxAvatarLength); - }); + } + + /** + * Handler to invoke IAvatarDropDownAction instance when the drop down option is selected + * @param {IAvatar} avatar the avatar item selected from the list + * @param {(IDropDownOption | void)} selectedOption drop down option selected + * @memberof StackedAvatarsList + */ + @action + onAvatarOptionSelected(avatar: IAvatar, selectedOption: IDropDownOption | void): void { + const { value } = selectedOption || { value: (a: IAvatar) => a }; + + value(avatar); + } } diff --git a/wherehows-web/app/components/datasets/containers/dataset-owner-list.ts b/wherehows-web/app/components/datasets/containers/dataset-owner-list.ts index c287dee9ec..5a3dbe00b9 100644 --- a/wherehows-web/app/components/datasets/containers/dataset-owner-list.ts +++ b/wherehows-web/app/components/datasets/containers/dataset-owner-list.ts @@ -1,18 +1,20 @@ import Component from '@ember/component'; import { set } from '@ember/object'; -import { task } from 'ember-concurrency'; -import { assert } from '@ember/debug'; +import { classNames } from '@ember-decorators/component'; import { computed } from '@ember-decorators/object'; +import { assert } from '@ember/debug'; +import { task } from 'ember-concurrency'; import { readDatasetOwnersByUrn } from 'wherehows-web/utils/api/datasets/owners'; -import { arrayMap } from 'wherehows-web/utils/array'; +import { arrayMap, arrayPipe } from 'wherehows-web/utils/array'; import { IAvatar } from 'wherehows-web/typings/app/avatars'; import { IOwner, IOwnerResponse } from 'wherehows-web/typings/api/datasets/owners'; import { getAvatarProps } from 'wherehows-web/constants/avatars/avatars'; -import { confirmedOwners } from 'wherehows-web/constants/datasets/owner'; +import { confirmedOwners, avatarWithDropDownOption } from 'wherehows-web/constants/datasets/owner'; import { containerDataSource } from 'wherehows-web/utils/components/containers/data-source'; import { isLiUrn } from 'wherehows-web/utils/validators/urn'; import { IAppConfig } from 'wherehows-web/typings/api/configurator/configurator'; +@classNames('dataset-owner-list') @containerDataSource('getOwnersTask') export default class DatasetOwnerListContainer extends Component { constructor() { @@ -57,7 +59,7 @@ export default class DatasetOwnerListContainer extends Component { @computed('owners') get avatars(): Array { const { avatarEntityProps, owners } = this; - return arrayMap(getAvatarProps(avatarEntityProps))(owners); + return arrayPipe(arrayMap(getAvatarProps(avatarEntityProps)), arrayMap(avatarWithDropDownOption))(owners); } /** diff --git a/wherehows-web/app/constants/datasets/owner.ts b/wherehows-web/app/constants/datasets/owner.ts index fcb431e822..aae4a8a333 100644 --- a/wherehows-web/app/constants/datasets/owner.ts +++ b/wherehows-web/app/constants/datasets/owner.ts @@ -2,6 +2,8 @@ import { set } from '@ember/object'; import { IOwner } from 'wherehows-web/typings/api/datasets/owners'; import { OwnerIdType, OwnerSource, OwnerType, OwnerUrnNamespace } from 'wherehows-web/utils/api/datasets/owners'; import { arrayFilter, isListUnique } from 'wherehows-web/utils/array'; +import { IAvatar } from 'wherehows-web/typings/app/avatars'; +import { buildMailToUrl } from 'wherehows-web/utils/helpers/email'; /** * Initial user name for candidate owners @@ -150,6 +152,27 @@ const confirmedOwners = arrayFilter(isConfirmedOwner); const isRequiredMinOwnersNotConfirmed = (owners: Array = []): boolean => validConfirmedOwners(owners).length < minRequiredConfirmedOwners; +/** + * Augments an owner instance of IAvatar requiring property avatarOptions to exist on the returned instance + * @param {IAvatar} avatar the avatar object to augment + * @returns {(IAvatar & Required>)} + */ +const avatarWithDropDownOption = (avatar: IAvatar): IAvatar & Required> => { + const email = avatar.email || ''; + + return { + ...avatar, + avatarOptions: [ + { + // if the owner avatar does not have an email then a null value is returned with no action performed + value: ({ email = '' }: IAvatar): Window | null => + email ? window.open(buildMailToUrl({ to: email || '' }), '_blank') : null, + label: email + } + ] + }; +}; + export { defaultOwnerProps, defaultOwnerUserName, @@ -160,5 +183,6 @@ export { updateOwner, confirmOwner, isConfirmedOwner, - confirmedOwners + confirmedOwners, + avatarWithDropDownOption }; diff --git a/wherehows-web/app/styles/components/_all.scss b/wherehows-web/app/styles/components/_all.scss index d952da22ed..8c2c42801d 100644 --- a/wherehows-web/app/styles/components/_all.scss +++ b/wherehows-web/app/styles/components/_all.scss @@ -29,6 +29,7 @@ @import 'dataset-health/all'; @import 'search/all'; @import 'hotkey/all'; +@import 'dataset-owner/all'; @import 'nacho/nacho-button'; @import 'nacho/nacho-global-search'; diff --git a/wherehows-web/app/styles/components/avatar/_avatar.scss b/wherehows-web/app/styles/components/avatar/_avatar.scss index ed38dbb521..a8ba132c81 100644 --- a/wherehows-web/app/styles/components/avatar/_avatar.scss +++ b/wherehows-web/app/styles/components/avatar/_avatar.scss @@ -5,11 +5,27 @@ .avatar-container { display: flex; - justify-content: flex-end; + justify-content: flex-start; + padding: item-spacing(3 0); + + /// --rtl modifier allows avatars to be animated from right-to-left + /// avatar-container element should ideally be positioned against the right edge of it's container + &--rtl { + justify-content: flex-end; + + &:hover { + .avatar-item--stacked { + margin-left: item-spacing(7) / 2; + transition-duration: 167ms; + transition-timing-function: cubic-bezier(0, 0, 0.2, 1); + transition-delay: 0s; + } + } + } &:hover { .avatar-item--stacked { - margin-left: item-spacing(7) / 2; + margin-right: item-spacing(7) / 2; transition-duration: 167ms; transition-timing-function: cubic-bezier(0, 0, 0.2, 1); transition-delay: 0s; @@ -18,6 +34,8 @@ &__list { list-style-type: none; + padding: 0; + margin: 0; } } @@ -52,6 +70,10 @@ } } } + + &--trigger { + outline: none; + } } .avatar { @@ -71,6 +93,10 @@ margin-left: -(item-spacing(7) / 2); } } + + &--focused { + border-color: get-color(blue5); + } } .avatar-rollup { diff --git a/wherehows-web/app/styles/components/dataset-owner/_all.scss b/wherehows-web/app/styles/components/dataset-owner/_all.scss new file mode 100644 index 0000000000..96985473b8 --- /dev/null +++ b/wherehows-web/app/styles/components/dataset-owner/_all.scss @@ -0,0 +1 @@ +@import 'owners'; diff --git a/wherehows-web/app/styles/components/dataset-owner/_owners.scss b/wherehows-web/app/styles/components/dataset-owner/_owners.scss new file mode 100644 index 0000000000..9d8de0a7a9 --- /dev/null +++ b/wherehows-web/app/styles/components/dataset-owner/_owners.scss @@ -0,0 +1,5 @@ +.dataset-owner-list { + display: inline-flex; + align-items: center; + margin: item-spacing(1 0); +} diff --git a/wherehows-web/app/styles/layout/_dataset.scss b/wherehows-web/app/styles/layout/_dataset.scss index 3ccf967cf7..db987bac7d 100644 --- a/wherehows-web/app/styles/layout/_dataset.scss +++ b/wherehows-web/app/styles/layout/_dataset.scss @@ -22,12 +22,4 @@ right: item-spacing(5); bottom: 0; } - - .dataset-owner-list { - margin-left: item-spacing(4); - - .avatar-container { - justify-content: flex-start; - } - } } diff --git a/wherehows-web/app/templates/components/avatars/stacked-avatars-list.hbs b/wherehows-web/app/templates/components/avatars/stacked-avatars-list.hbs index d163e67a31..ebcb9717c9 100644 --- a/wherehows-web/app/templates/components/avatars/stacked-avatars-list.hbs +++ b/wherehows-web/app/templates/components/avatars/stacked-avatars-list.hbs @@ -14,30 +14,28 @@
    {{#each maxAvatars as |avatar|}}
  • + {{#nacho/dropdown/hover-dropdown + dropDownItems=avatar.avatarOptions + selectedDropDown=null + wrappedTriggerComponent=true + renderInPlace=true + onSelect=(action "onAvatarOptionSelected" avatar) as |dd|}} - {{avatars/avatar-image avatar=avatar class="avatar--stacked"}} + {{#dd.trigger class="avatar-item--trigger"}} + {{avatars/avatar-image avatar=avatar + class=(if dd.isExpanded "avatar--stacked avatar--focused" "avatar--stacked")}} + {{/dd.trigger}} - {{#avatars/avatar-metadata class="avatar-item--stacked__meta" avatar=avatar as |aviMeta|}} - {{#if aviMeta.email}} - - {{aviMeta.email}} - - {{/if}} - - {{#if aviMeta.userName}} - - {{aviMeta.userName}} - - {{/if}} - {{/avatars/avatar-metadata}} + {{dd.content}} + {{/nacho/dropdown/hover-dropdown}}
    • {{/each}}
{{#if rollupAvatars}} - {{#avatars/rollup-avatars avatars=avatars avatarType=avatarType}} - {{rollupAvatars.length}}+ + {{#avatars/rollup-avatars avatars=avatars avatarType=@avatarType}} + +{{rollupAvatars.length}} {{/avatars/rollup-avatars}} {{/if}} diff --git a/wherehows-web/app/templates/components/nacho/dropdown/hover-dropdown.hbs b/wherehows-web/app/templates/components/nacho/dropdown/hover-dropdown.hbs index 4acc6750f5..523454f4db 100644 --- a/wherehows-web/app/templates/components/nacho/dropdown/hover-dropdown.hbs +++ b/wherehows-web/app/templates/components/nacho/dropdown/hover-dropdown.hbs @@ -1,20 +1,28 @@ -{{#basic-dropdown as |bd|}} - {{#bd.trigger class="nacho-drop-down__trigger" - onMouseDown=(action "prevent") - onMouseEnter=(action "showDropDown") - onMouseLeave=(action "hideDropDown")}} - - {{selectedDropDown.label}} - +{{#basic-dropdown renderInPlace=@renderInPlace as |bd|}} + {{#if (not-eq @wrappedTriggerComponent true)}} + {{#bd.trigger class="nacho-drop-down__trigger" + onMouseDown=(action "prevent") + onMouseEnter=(action "showDropDown") + onMouseLeave=(action "hideDropDown")}} + + {{selectedDropDown.label}} + - {{#if selectedDropDown}} - - {{fa-icon (if isExpanded "caret-up" "caret-down")}} - - {{/if}} - {{/bd.trigger}} + {{#if selectedDropDown}} + + {{fa-icon (if isExpanded "caret-up" "caret-down")}} + + {{/if}} + {{/bd.trigger}} + {{/if}} {{yield (hash + trigger=(component bd.trigger + onMouseDown=(action "prevent") + onMouseEnter=(action "showDropDown") + onMouseLeave=(action "hideDropDown") + ) + isExpanded=isExpanded content=(component 'nacho/dropdown/dropdown-content' baseComponent=bd.content onMouseEnter=(action "showDropDown") diff --git a/wherehows-web/app/templates/datasets/dataset.hbs b/wherehows-web/app/templates/datasets/dataset.hbs index e4d692b08b..f2d346a13e 100644 --- a/wherehows-web/app/templates/datasets/dataset.hbs +++ b/wherehows-web/app/templates/datasets/dataset.hbs @@ -56,16 +56,14 @@ {{/datasets/containers/dataset-fabrics}} - {{datasets/containers/dataset-owner-list - urn=encodedUrn - avatarEntityProps=avatarEntityProps - class="dataset-owner-list"}} - {{#link-to "datasets.dataset.health" encodedUrn}} {{datasets/containers/health-score-gauge urn=encodedUrn wikiLinks=wikiLinks}} {{/link-to}} + {{datasets/containers/dataset-owner-list + urn=encodedUrn + avatarEntityProps=avatarEntityProps}} diff --git a/wherehows-web/app/typings/app/avatars.d.ts b/wherehows-web/app/typings/app/avatars.d.ts index 601ecfb123..a5f3d08284 100644 --- a/wherehows-web/app/typings/app/avatars.d.ts +++ b/wherehows-web/app/typings/app/avatars.d.ts @@ -1,3 +1,13 @@ +import { IDropDownOption } from 'wherehows-web/typings/app/dataset-compliance'; + +/** + * Defines the interface for functions that are supplied as options to IAvatar.avatarOptions + * @interface IAvatarDropDownAction + */ +interface IAvatarDropDownAction { + (avatar: IAvatar): any; +} + /** * Describes the interface for an avatar object * @interface IAvatar @@ -11,6 +21,8 @@ interface IAvatar { // Handle for the avatar userName?: string; name?: string; + // Selection options for an avatar with dropdown + avatarOptions?: Array>; } -export { IAvatar }; +export { IAvatar, IAvatarDropDownAction }; diff --git a/wherehows-web/app/typings/app/core.d.ts b/wherehows-web/app/typings/app/core.d.ts new file mode 100644 index 0000000000..7d5c2c2aec --- /dev/null +++ b/wherehows-web/app/typings/app/core.d.ts @@ -0,0 +1,14 @@ +/** + * Defines a generic type for a type T that could also be undefined + * @template T type which maybe is generic over + */ +type Maybe = T | undefined; + +/** + * Defines a generic type that recasts a type T as a union of U and an intersection of T and U + * @template T the type to be recast + * @template U the result of the type recast + */ +type Recast = T & U | U; + +export { Maybe, Recast }; diff --git a/wherehows-web/app/typings/app/helpers/email.d.ts b/wherehows-web/app/typings/app/helpers/email.d.ts new file mode 100644 index 0000000000..c782c4f428 --- /dev/null +++ b/wherehows-web/app/typings/app/helpers/email.d.ts @@ -0,0 +1,18 @@ +import { Maybe } from 'wherehows-web/typings/app/core'; + +/** + * Union string literal of safe email headers and the body field + */ +type MailerHeaders = 'to' | 'cc' | 'bcc' | 'subject' | 'body'; + +/** + * Alias for a string of list of string email header values + * @alias + */ +export type MailerHeaderValue = Maybe>; + +/** + * An optional record of email headers to it's value - MailerHeaderValue + * @alias + */ +export type IMailHeaderRecord = Partial>; diff --git a/wherehows-web/app/utils/helpers/email.ts b/wherehows-web/app/utils/helpers/email.ts new file mode 100644 index 0000000000..bcfd9fe790 --- /dev/null +++ b/wherehows-web/app/utils/helpers/email.ts @@ -0,0 +1,22 @@ +import { arrayReduce } from 'wherehows-web/utils/array'; +import buildUrl from 'wherehows-web/utils/build-url'; +import { IMailHeaderRecord, MailerHeaderValue } from 'wherehows-web/typings/app/helpers/email'; + +/** + * Constructs a `mailto:` address with supplied email headers as query parameters + * @param {IMailHeaderRecord} [headers={}] + * @returns {string} + */ +const buildMailToUrl = (headers: IMailHeaderRecord = {}): string => { + const { to = '', ...otherHeaders } = headers; + const [...otherHeaderPairs] = Object.entries(otherHeaders); + const mailTo = `mailto:${to}`; + + return arrayReduce( + (mailTo: string, [headerName, headerValue = '']: [string, MailerHeaderValue]) => + buildUrl(mailTo, headerName, String(headerValue)), + mailTo + )([...otherHeaderPairs]); +}; + +export { buildMailToUrl }; diff --git a/wherehows-web/tests/helpers/validators/email/email-test-fixture.ts b/wherehows-web/tests/helpers/validators/email/email-test-fixture.ts new file mode 100644 index 0000000000..b19a2fbc60 --- /dev/null +++ b/wherehows-web/tests/helpers/validators/email/email-test-fixture.ts @@ -0,0 +1,59 @@ +const emailAddress = 'nowhere@example.com'; +const cc = 'nowhere1@example.com'; +const bcc = 'nowhere-bcc@example.com'; +const body = 'Message'; +const subject = 'Email'; +const base = 'mailto:'; +const mailToEmail = `${base}${emailAddress}`; +const emailMailToAsserts = [ + { + __expected__: base, + __args__: void 0, + __assert_msg__: 'it should return a basic mailto: string without an email when no arguments are passed' + }, + { + __expected__: base, + __args__: {}, + __assert_msg__: 'it should return a basic mailto: string without an email when an empty object is passed' + }, + { + __expected__: base, + __args__: { to: '' }, + __assert_msg__: + 'it should return a basic mailto: string without an email when an object with only an empty string in the `to` field is passed' + }, + { + __expected__: `${mailToEmail}`, + __args__: { to: emailAddress }, + __assert_msg__: 'it should return a mailto: string with an email when an object with only the `to` field is passed' + }, + { + __expected__: `${mailToEmail}?cc=${encodeURIComponent(cc)}`, + __args__: { to: emailAddress, cc }, + __assert_msg__: 'it should return a mailto: string with an email and a cc query when to and cc are passed in' + }, + { + __expected__: `${mailToEmail}?cc=${encodeURIComponent(cc)}&subject=${encodeURIComponent(subject)}`, + __args__: { to: emailAddress, cc, subject }, + __assert_msg__: + 'it should return a mailto: string with an email, subject, and a cc query when to, subject, and cc are passed in' + }, + { + __expected__: `${mailToEmail}?cc=${encodeURIComponent(cc)}&subject=${encodeURIComponent( + subject + )}&bcc=${encodeURIComponent(bcc)}`, + __args__: { to: emailAddress, cc, subject, bcc }, + __assert_msg__: + 'it should return a mailto: string with an email, subject, bcc, and a cc query when to, subject, bcc, and cc are passed in' + }, + { + __expected__: `${mailToEmail}?cc=${encodeURIComponent(cc)}&subject=${encodeURIComponent( + subject + )}&bcc=${encodeURIComponent(bcc)}&body=${encodeURIComponent(body)}`, + __args__: { to: emailAddress, cc, subject, bcc, body }, + __assert_msg__: + 'it should return a mailto: string with an email, subject, bcc, body, and a cc query when to, subject, bcc, body, and cc are passed in' + } +]; + +export { emailMailToAsserts }; diff --git a/wherehows-web/tests/unit/utils/helpers/email-test.ts b/wherehows-web/tests/unit/utils/helpers/email-test.ts new file mode 100644 index 0000000000..f89ee72cc1 --- /dev/null +++ b/wherehows-web/tests/unit/utils/helpers/email-test.ts @@ -0,0 +1,17 @@ +import { buildMailToUrl } from 'wherehows-web/utils/helpers/email'; +import { module, test } from 'qunit'; +import { emailMailToAsserts } from 'wherehows-web/tests/helpers/validators/email/email-test-fixture'; + +module('Unit | Utility | helpers/email', function(): void { + test('buildMailToUrl exists', function(assert) { + assert.equal(typeof buildMailToUrl, 'function', 'it is of function type'); + }); + + test('buildMailToUrl generates expected url values', function(assert): void { + assert.expect(emailMailToAsserts.length); + + emailMailToAsserts.forEach(({ __expected__, __args__, __assert_msg__ }) => { + assert.equal(buildMailToUrl(__args__), __expected__, __assert_msg__); + }); + }); +}); From 10e8fdec8aba8820ea2616ac706b61f4b698eca2 Mon Sep 17 00:00:00 2001 From: Seyi Adebajo Date: Wed, 26 Sep 2018 21:57:16 -0700 Subject: [PATCH 2/3] refactors email for avatars --- .../datasets/containers/dataset-owner-list.ts | 11 ++- wherehows-web/app/constants/datasets/owner.ts | 2 +- .../app/styles/components/avatar/_avatar.scss | 2 +- .../avatars/stacked-avatars-list.hbs | 2 +- .../app/typings/app/helpers/email.d.ts | 2 +- wherehows-web/app/utils/build-url.ts | 68 ++++++++++++------- wherehows-web/app/utils/helpers/email.ts | 12 +--- .../validators/email/email-test-fixture.ts | 52 +++++++------- .../tests/unit/utils/build-url-test.ts | 36 +++++++++- .../tests/unit/utils/helpers/email-test.ts | 4 +- 10 files changed, 117 insertions(+), 74 deletions(-) diff --git a/wherehows-web/app/components/datasets/containers/dataset-owner-list.ts b/wherehows-web/app/components/datasets/containers/dataset-owner-list.ts index 5a3dbe00b9..a6eb1d12a5 100644 --- a/wherehows-web/app/components/datasets/containers/dataset-owner-list.ts +++ b/wherehows-web/app/components/datasets/containers/dataset-owner-list.ts @@ -11,7 +11,7 @@ import { IOwner, IOwnerResponse } from 'wherehows-web/typings/api/datasets/owner import { getAvatarProps } from 'wherehows-web/constants/avatars/avatars'; import { confirmedOwners, avatarWithDropDownOption } from 'wherehows-web/constants/datasets/owner'; import { containerDataSource } from 'wherehows-web/utils/components/containers/data-source'; -import { isLiUrn } from 'wherehows-web/utils/validators/urn'; +import { decodeUrn, isLiUrn } from 'wherehows-web/utils/validators/urn'; import { IAppConfig } from 'wherehows-web/typings/api/configurator/configurator'; @classNames('dataset-owner-list') @@ -59,7 +59,12 @@ export default class DatasetOwnerListContainer extends Component { @computed('owners') get avatars(): Array { const { avatarEntityProps, owners } = this; - return arrayPipe(arrayMap(getAvatarProps(avatarEntityProps)), arrayMap(avatarWithDropDownOption))(owners); + const [getAvatarProperties, augmentAvatarsWithDropDownOption] = [ + arrayMap(getAvatarProps(avatarEntityProps)), + arrayMap(avatarWithDropDownOption) + ]; + + return arrayPipe(getAvatarProperties, augmentAvatarsWithDropDownOption)(owners); } /** @@ -69,7 +74,7 @@ export default class DatasetOwnerListContainer extends Component { getOwnersTask = task(function*(this: DatasetOwnerListContainer): IterableIterator> { const { urn } = this; - if (isLiUrn(urn)) { + if (isLiUrn(decodeUrn(urn))) { const { owners = [] }: IOwnerResponse = yield readDatasetOwnersByUrn(urn); set(this, 'owners', confirmedOwners(owners)); diff --git a/wherehows-web/app/constants/datasets/owner.ts b/wherehows-web/app/constants/datasets/owner.ts index aae4a8a333..90860f17c1 100644 --- a/wherehows-web/app/constants/datasets/owner.ts +++ b/wherehows-web/app/constants/datasets/owner.ts @@ -165,7 +165,7 @@ const avatarWithDropDownOption = (avatar: IAvatar): IAvatar & Required + value: ({ email }: IAvatar): Window | null => email ? window.open(buildMailToUrl({ to: email || '' }), '_blank') : null, label: email } diff --git a/wherehows-web/app/styles/components/avatar/_avatar.scss b/wherehows-web/app/styles/components/avatar/_avatar.scss index a8ba132c81..30669e0645 100644 --- a/wherehows-web/app/styles/components/avatar/_avatar.scss +++ b/wherehows-web/app/styles/components/avatar/_avatar.scss @@ -71,7 +71,7 @@ } } - &--trigger { + &__trigger { outline: none; } } diff --git a/wherehows-web/app/templates/components/avatars/stacked-avatars-list.hbs b/wherehows-web/app/templates/components/avatars/stacked-avatars-list.hbs index ebcb9717c9..00b7077e81 100644 --- a/wherehows-web/app/templates/components/avatars/stacked-avatars-list.hbs +++ b/wherehows-web/app/templates/components/avatars/stacked-avatars-list.hbs @@ -21,7 +21,7 @@ renderInPlace=true onSelect=(action "onAvatarOptionSelected" avatar) as |dd|}} - {{#dd.trigger class="avatar-item--trigger"}} + {{#dd.trigger class="avatar-item__trigger"}} {{avatars/avatar-image avatar=avatar class=(if dd.isExpanded "avatar--stacked avatar--focused" "avatar--stacked")}} {{/dd.trigger}} diff --git a/wherehows-web/app/typings/app/helpers/email.d.ts b/wherehows-web/app/typings/app/helpers/email.d.ts index c782c4f428..8347059885 100644 --- a/wherehows-web/app/typings/app/helpers/email.d.ts +++ b/wherehows-web/app/typings/app/helpers/email.d.ts @@ -9,7 +9,7 @@ type MailerHeaders = 'to' | 'cc' | 'bcc' | 'subject' | 'body'; * Alias for a string of list of string email header values * @alias */ -export type MailerHeaderValue = Maybe>; +export type MailerHeaderValue = Maybe; /** * An optional record of email headers to it's value - MailerHeaderValue diff --git a/wherehows-web/app/utils/build-url.ts b/wherehows-web/app/utils/build-url.ts index e7cb51cc57..d55ee1e19c 100644 --- a/wherehows-web/app/utils/build-url.ts +++ b/wherehows-web/app/utils/build-url.ts @@ -1,18 +1,24 @@ import { encode, decode } from 'wherehows-web/utils/encode-decode-uri-component-with-space'; import { isBlank } from '@ember/utils'; +import { isObject } from 'wherehows-web/utils/object'; /** * Construct a url by appending a query pair (?key=value | &key=value) to a base url and * encoding the query value in the pair - * @param {String} baseUrl the base or original url that will be appended with a query string - * @param {String} queryParam - * @param {String} queryValue + * @param baseUrl the base or original url that will be appended with a query string + * @param queryParamOrMap a map of query keys to values or a single query param key + * @param queryValue if a queryParam is supplied, then a queryValue can be expected + * @param useEncoding flag indicating if the query values should be encoded * @returns {string} */ -function buildUrl(): string; -function buildUrl(baseUrl: string, mapParams: Record): string; -function buildUrl(baseUrl: string, queryKey: string, queryValue: string): string; -function buildUrl(baseUrl?: string, queryParamOrMap?: string | Record, queryValue?: string): string { +function buildUrl(baseUrl: string, queryParamOrMap?: {}, useEncoding?: boolean): string; +function buildUrl(baseUrl: string, queryParamOrMap?: string, queryValue?: string, useEncoding?: boolean): string; +function buildUrl( + baseUrl: string, + queryParamOrMap: string | Record = {}, + queryValue?: string | boolean, + useEncoding: boolean = true +): string { if (!baseUrl) { return ''; } @@ -21,16 +27,24 @@ function buildUrl(baseUrl?: string, queryParamOrMap?: string | Record = {}; + + // queryParamOrMap is a string then, reify paramMap object with supplied value if (typeof queryParamOrMap === 'string') { paramMap = { - [queryParamOrMap]: queryValue || '' + [queryParamOrMap]: queryValue }; - } else { - paramMap = queryParamOrMap; } - return Object.keys(paramMap).reduce((url, paramKey) => { + if (isObject(queryParamOrMap)) { + paramMap = queryParamOrMap; + + if (typeof queryValue === 'boolean') { + useEncoding = queryValue; + } + } + + return Object.keys(paramMap).reduce((url: string, paramKey: string): string => { // If the query string already contains the initial question mark append // kv-pair with ampersand const separator = String(url).includes('?') ? '&' : '?'; @@ -44,21 +58,23 @@ function buildUrl(baseUrl?: string, queryParamOrMap?: string | Record { const { to = '', ...otherHeaders } = headers; - const [...otherHeaderPairs] = Object.entries(otherHeaders); - const mailTo = `mailto:${to}`; + const mailTo = `mailto:${encodeURIComponent(to)}`; - return arrayReduce( - (mailTo: string, [headerName, headerValue = '']: [string, MailerHeaderValue]) => - buildUrl(mailTo, headerName, String(headerValue)), - mailTo - )([...otherHeaderPairs]); + return buildUrl(mailTo, otherHeaders); }; export { buildMailToUrl }; diff --git a/wherehows-web/tests/helpers/validators/email/email-test-fixture.ts b/wherehows-web/tests/helpers/validators/email/email-test-fixture.ts index b19a2fbc60..8ddd6538f3 100644 --- a/wherehows-web/tests/helpers/validators/email/email-test-fixture.ts +++ b/wherehows-web/tests/helpers/validators/email/email-test-fixture.ts @@ -7,51 +7,47 @@ const base = 'mailto:'; const mailToEmail = `${base}${emailAddress}`; const emailMailToAsserts = [ { - __expected__: base, - __args__: void 0, - __assert_msg__: 'it should return a basic mailto: string without an email when no arguments are passed' + expected: base, + args: void 0, + assertMsg: 'it should return a basic mailto: string without an email when no arguments are passed' }, { - __expected__: base, - __args__: {}, - __assert_msg__: 'it should return a basic mailto: string without an email when an empty object is passed' + expected: base, + args: {}, + assertMsg: 'it should return a basic mailto: string without an email when an empty object is passed' }, { - __expected__: base, - __args__: { to: '' }, - __assert_msg__: + expected: base, + args: { to: '' }, + assertMsg: 'it should return a basic mailto: string without an email when an object with only an empty string in the `to` field is passed' }, { - __expected__: `${mailToEmail}`, - __args__: { to: emailAddress }, - __assert_msg__: 'it should return a mailto: string with an email when an object with only the `to` field is passed' + expected: `${mailToEmail}`, + args: { to: emailAddress }, + assertMsg: 'it should return a mailto: string with an email when an object with only the `to` field is passed' }, { - __expected__: `${mailToEmail}?cc=${encodeURIComponent(cc)}`, - __args__: { to: emailAddress, cc }, - __assert_msg__: 'it should return a mailto: string with an email and a cc query when to and cc are passed in' + expected: `${mailToEmail}?cc=${cc}`, + args: { to: emailAddress, cc }, + assertMsg: 'it should return a mailto: string with an email and a cc query when to and cc are passed in' }, { - __expected__: `${mailToEmail}?cc=${encodeURIComponent(cc)}&subject=${encodeURIComponent(subject)}`, - __args__: { to: emailAddress, cc, subject }, - __assert_msg__: + expected: `${mailToEmail}?cc=${cc}&subject=${subject}`, + args: { to: emailAddress, cc, subject }, + assertMsg: 'it should return a mailto: string with an email, subject, and a cc query when to, subject, and cc are passed in' }, { - __expected__: `${mailToEmail}?cc=${encodeURIComponent(cc)}&subject=${encodeURIComponent( - subject - )}&bcc=${encodeURIComponent(bcc)}`, - __args__: { to: emailAddress, cc, subject, bcc }, - __assert_msg__: + expected: `${mailToEmail}?cc=${cc}&subject=${subject}&bcc=${bcc}`, + args: { to: emailAddress, cc, subject, bcc }, + assertMsg: 'it should return a mailto: string with an email, subject, bcc, and a cc query when to, subject, bcc, and cc are passed in' }, { - __expected__: `${mailToEmail}?cc=${encodeURIComponent(cc)}&subject=${encodeURIComponent( - subject - )}&bcc=${encodeURIComponent(bcc)}&body=${encodeURIComponent(body)}`, - __args__: { to: emailAddress, cc, subject, bcc, body }, - __assert_msg__: + expected: `${mailToEmail}?cc=${cc}&subject=${subject}&bcc=${bcc}&body=${body}`, + args: { to: emailAddress, cc, subject, bcc, body }, + assertMsg: 'it should return a mailto: string with an email, subject, bcc, body, and a cc query when to, subject, bcc, body, and cc are passed in' } ]; diff --git a/wherehows-web/tests/unit/utils/build-url-test.ts b/wherehows-web/tests/unit/utils/build-url-test.ts index 7bbc49d5c4..2f42586cab 100644 --- a/wherehows-web/tests/unit/utils/build-url-test.ts +++ b/wherehows-web/tests/unit/utils/build-url-test.ts @@ -3,10 +3,42 @@ import { module, test } from 'qunit'; module('Unit | Utility | build url', function() { const baseUrl = 'https://www.linkedin.com'; + const unEncodedString = 'string@string'; + const encodedString = encodeURIComponent(unEncodedString); test('baseUrl', function(assert) { - let result = buildUrl(); - assert.equal(result, '', 'returns an empty string when no arguments are passed'); + let result = buildUrl(baseUrl); + assert.equal(result, baseUrl, 'it returns the base url when no other arguments are passed'); + + result = buildUrl(baseUrl, {}); + assert.equal(result, baseUrl, 'it returns the base url when an empty object is supplied as query parameter'); + + result = buildUrl(baseUrl, ''); + assert.equal(result, baseUrl, 'it returns the base url when an empty string is supplied as query parameter'); + + result = buildUrl(baseUrl, '', true); + assert.equal( + result, + baseUrl, + 'it returns the base url when the queryParam is an empty string and the useEncoding is set' + ); + + result = buildUrl(baseUrl, 'query', true); + assert.equal( + result, + `${baseUrl}?query=true`, + 'it returns the base url with a query key set to true when true is passed as a query value' + ); + + result = buildUrl(baseUrl, 'query', unEncodedString, true); + assert.equal( + result, + `${baseUrl}?query=${encodedString}`, + 'it returns the encoded value when the useEncoding flag is true' + ); + + result = buildUrl(baseUrl, { query: unEncodedString }, true); + assert.equal(result, `${baseUrl}?query=${encodedString}`, 'it returns the encoded string on a queryParams object'); result = buildUrl(baseUrl, '', ''); assert.equal(result, baseUrl, 'returns the baseUrl when no query parameter is supplied'); diff --git a/wherehows-web/tests/unit/utils/helpers/email-test.ts b/wherehows-web/tests/unit/utils/helpers/email-test.ts index f89ee72cc1..860108ad63 100644 --- a/wherehows-web/tests/unit/utils/helpers/email-test.ts +++ b/wherehows-web/tests/unit/utils/helpers/email-test.ts @@ -10,8 +10,8 @@ module('Unit | Utility | helpers/email', function(): void { test('buildMailToUrl generates expected url values', function(assert): void { assert.expect(emailMailToAsserts.length); - emailMailToAsserts.forEach(({ __expected__, __args__, __assert_msg__ }) => { - assert.equal(buildMailToUrl(__args__), __expected__, __assert_msg__); + emailMailToAsserts.forEach(({ expected, args, assertMsg }) => { + assert.equal(buildMailToUrl(args), expected, assertMsg); }); }); }); From 4e50b07d3ec258e3086b6515963fd5eac4027fef Mon Sep 17 00:00:00 2001 From: Seyi Adebajo Date: Wed, 26 Sep 2018 22:20:23 -0700 Subject: [PATCH 3/3] fix failing tests for buildMailToUrl --- .../helpers/validators/email/email-test-fixture.ts | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/wherehows-web/tests/helpers/validators/email/email-test-fixture.ts b/wherehows-web/tests/helpers/validators/email/email-test-fixture.ts index 8ddd6538f3..85120a0aa5 100644 --- a/wherehows-web/tests/helpers/validators/email/email-test-fixture.ts +++ b/wherehows-web/tests/helpers/validators/email/email-test-fixture.ts @@ -4,7 +4,7 @@ const bcc = 'nowhere-bcc@example.com'; const body = 'Message'; const subject = 'Email'; const base = 'mailto:'; -const mailToEmail = `${base}${emailAddress}`; +const mailToEmail = `${base}${encodeURIComponent(emailAddress)}`; const emailMailToAsserts = [ { expected: base, @@ -28,24 +28,28 @@ const emailMailToAsserts = [ assertMsg: 'it should return a mailto: string with an email when an object with only the `to` field is passed' }, { - expected: `${mailToEmail}?cc=${cc}`, + expected: `${mailToEmail}?cc=${encodeURIComponent(cc)}`, args: { to: emailAddress, cc }, assertMsg: 'it should return a mailto: string with an email and a cc query when to and cc are passed in' }, { - expected: `${mailToEmail}?cc=${cc}&subject=${subject}`, + expected: `${mailToEmail}?cc=${encodeURIComponent(cc)}&subject=${encodeURIComponent(subject)}`, args: { to: emailAddress, cc, subject }, assertMsg: 'it should return a mailto: string with an email, subject, and a cc query when to, subject, and cc are passed in' }, { - expected: `${mailToEmail}?cc=${cc}&subject=${subject}&bcc=${bcc}`, + expected: `${mailToEmail}?cc=${encodeURIComponent(cc)}&subject=${encodeURIComponent( + subject + )}&bcc=${encodeURIComponent(bcc)}`, args: { to: emailAddress, cc, subject, bcc }, assertMsg: 'it should return a mailto: string with an email, subject, bcc, and a cc query when to, subject, bcc, and cc are passed in' }, { - expected: `${mailToEmail}?cc=${cc}&subject=${subject}&bcc=${bcc}&body=${body}`, + expected: `${mailToEmail}?cc=${encodeURIComponent(cc)}&subject=${encodeURIComponent( + subject + )}&bcc=${encodeURIComponent(bcc)}&body=${encodeURIComponent(body)}`, args: { to: emailAddress, cc, subject, bcc, body }, assertMsg: 'it should return a mailto: string with an email, subject, bcc, body, and a cc query when to, subject, bcc, body, and cc are passed in'