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

Add ability to show component in alert #1118

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Add ability to show component in alert
  • Loading branch information
matthew-white committed Dec 20, 2024
commit f7140ec2ae0128624ce41186cbfe9abb402fe444
53 changes: 32 additions & 21 deletions src/alert.js
Original file line number Diff line number Diff line change
@@ -12,40 +12,51 @@ except according to the terms contained in the LICENSE file.
import { shallowReactive } from 'vue';

class AlertData {
#data;

constructor() {
this._data = shallowReactive({
this.#data = shallowReactive({
// The alert's "contextual" type: 'success', 'info', 'warning', or
// 'danger'.
type: 'danger',
message: null,
// `true` if the alert should be visible and `false` if not.
state: false,
// The time at which the alert was last set
// Either a string or an array with a component and props for the
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about having object instead of array? or add component and props getter so that alert.vue can access them without bracket notation.

// component
content: null,
// The time at which the alert was last shown
at: new Date()
});
}

get type() { return this._data.type; }
get message() { return this._data.message; }
get state() { return this._data.state; }
get at() { return this._data.at; }
// Returns `true` if the alert should be visible and `false` if not.
get state() { return this.content != null; }

get type() { return this.#data.type; }

get content() { return this.#data.content; }

_set(type, message) {
this._data.type = type;
this._data.message = message;
this._data.state = true;
this._data.at = new Date();
get message() {
const { content } = this.#data;
return typeof content === 'string' ? content : null;
}

success(message) { this._set('success', message); }
info(message) { this._set('info', message); }
warning(message) { this._set('warning', message); }
danger(message) { this._set('danger', message); }
get at() { return this.#data.at; }

blank() {
this._data.state = false;
this._data.message = null;
// `content` can be a string, an array with a component and props for the
// component, or just a component.
#show(type, content) {
this.#data.type = type;
this.#data.content = typeof content === 'string' || Array.isArray(content)
? content
: [content, {}];
this.#data.at = new Date();
}

success(content) { this.#show('success', content); }
info(content) { this.#show('info', content); }
warning(content) { this.#show('warning', content); }
danger(content) { this.#show('danger', content); }

blank() { this.#data.content = null; }
}

// Only a single alert is shown at a time. This function returns an object that
7 changes: 6 additions & 1 deletion src/components/alert.vue
Original file line number Diff line number Diff line change
@@ -16,7 +16,12 @@ except according to the terms contained in the LICENSE file.
@click="alert.blank()">
<span aria-hidden="true">&times;</span>
</button>
<span class="alert-message">{{ alert.message }}</span>

<span v-if="alert.message != null" class="alert-message">
{{ alert.message }}
</span>
<component v-else-if="alert.content != null" :is="alert.content[0]"
v-bind="alert.content[1]"/>
</div>
</template>

46 changes: 46 additions & 0 deletions src/components/alert/40917.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
<!--
Copyright 2024 ODK Central Developers
See the NOTICE file at the top-level directory of this distribution and at
https://github.com/getodk/central-frontend/blob/master/NOTICE.

This file is part of ODK Central. It is subject to the license terms in
the LICENSE file found in the top-level directory of this distribution and at
https://www.apache.org/licenses/LICENSE-2.0. No part of ODK Central,
including this file, may be copied, modified, propagated, or distributed
except according to the terms contained in the LICENSE file.
-->
<template>
<div>
<span>{{ $tc('message', duplicateProperties.length) }}</span>
<ul>
<li v-for="p of duplicateProperties" :key="p.provided">
{{ $t('duplicateProperty', p) }}
</li>
</ul>
</div>
</template>

<script setup>
defineOptions({
name: 'Alert40917'
Copy link
Member Author

Choose a reason for hiding this comment

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

I tried naming this Alert409_17, but the linter complained whenever I imported it:

Identifier 'Alert409_17' is not in camel case

Copy link
Contributor

Choose a reason for hiding this comment

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

other options to consider:

  • Alert409Dot17
  • AlertDuplicateProperties (semantic names)

});
defineProps({
duplicateProperties: {
type: Array,
required: true
}
});
</script>

<i18n lang="json5">
{
// @transifexKey util.request.problem.409_17
"en": {
"message": "This Form attempts to create a new Entity property that matches with an existing one except for capitalization: | This Form attempts to create new Entity properties that match with existing ones except for capitalization:",
// Error message format for the duplicate properties (different capitalization) in an Entity-list.
// {provided} is the property name in the uploaded Form.
// {current} is the existing property name in the Entity-list.
"duplicateProperty": "{provided} (existing: {current})"
}
}
</i18n>
9 changes: 1 addition & 8 deletions src/locales/en.json5
Original file line number Diff line number Diff line change
@@ -485,14 +485,7 @@
"problem": {
// A "resource" is a generic term for something in Central, for example,
// a Project, a Web User, or a Form.
"404_1": "The resource you are looking for cannot be found. The resource may have been deleted.",
"409_17": {
"message": "This Form attempts to create a new Entity property that matches with an existing one except for capitalization: | This Form attempts to create new Entity properties that match with existing ones except for capitalization:",
// Error message format for the duplicate properties (different capitalization) in an Entity-list.
// {provided} is the property name in the uploaded Form.
// {current} is the existing property name in the Entity-list.
"duplicateProperty": "{provided} (existing: {current})"
}
"404_1": "The resource you are looking for cannot be found. The resource may have been deleted."
}
},
"session": {
9 changes: 3 additions & 6 deletions src/util/request.js
Original file line number Diff line number Diff line change
@@ -9,6 +9,8 @@ https://www.apache.org/licenses/LICENSE-2.0. No part of ODK Central,
including this file, may be copied, modified, propagated, or distributed
except according to the terms contained in the LICENSE file.
*/
import Alert40917 from '../components/alert/40917.vue';

import { odataLiteral } from './odata';

// Returns `true` if `data` looks like a Backend Problem and `false` if not.
@@ -245,12 +247,7 @@ export const requestAlertMessage = (i18n, axiosError, problemToAlert = undefined
if (problem.code === 404.1) return i18n.t('util.request.problem.404_1');
if (problem.code === 409.17) {
const { duplicateProperties } = problem.details;
// eslint-disable-next-line prefer-template
return i18n.tc('util.request.problem.409_17.message', duplicateProperties.length) +
'\n\n' +
duplicateProperties
.map(p => `• ${i18n.t('util.request.problem.409_17.duplicateProperty', p)}`)
.join('\n');
return [Alert40917, { duplicateProperties }];
}
return problem.message;
};
17 changes: 14 additions & 3 deletions test/assertions.js
Original file line number Diff line number Diff line change
@@ -316,15 +316,26 @@ addAsyncMethod('textTooltip', async function textTooltip() {
////////////////////////////////////////////////////////////////////////////////
// OTHER

Assertion.addMethod('alert', function assertAlert(type = undefined, message = undefined) {
Assertion.addMethod('alert', function assertAlert(type = undefined, content = undefined) {
expect(this._obj).to.be.instanceof(VueWrapper);
const { alert } = this._obj.vm.$container;
this.assert(
alert.state,
'expected the component to show an alert',
'expected the component not to show an alert'
);
if (checkNegate(this, [type, message])) return;
if (checkNegate(this, [type, content])) return;
if (type != null) alert.type.should.equal(type);
if (message != null) alert.message.should.stringMatch(message);
if (content != null) {
if (typeof content === 'string' || content instanceof RegExp ||
typeof content === 'function') {
alert.message.should.stringMatch(content);
} else if (Array.isArray(content)) {
const [component, props = {}] = content;
alert.content[0].should.equal(component);
alert.content[1].should.eql(props);
} else {
alert.content.should.eql([content, {}]);
}
}
});
9 changes: 5 additions & 4 deletions test/components/form-draft/publish.spec.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { RouterLinkStub } from '@vue/test-utils';

import Alert40917 from '../../../src/components/alert/40917.vue';
import FormDraftPublish from '../../../src/components/form-draft/publish.vue';
import FormVersionRow from '../../../src/components/form-version/row.vue';

@@ -297,10 +298,10 @@ describe('FormDraftPublish', () => {
details: { duplicateProperties: [{ current: 'first_name', provided: 'FIRST_NAME' }] }
})
.afterResponse(modal => {
modal.should.alert(
'danger',
/This Form attempts to create a new Entity property that matches with an existing one except for capitalization:.*FIRST_NAME \(existing: first_name\)/s
);
modal.should.alert('danger', [
Alert40917,
{ duplicateProperties: [{ current: 'first_name', provided: 'FIRST_NAME' }] }
]);
});
});

9 changes: 5 additions & 4 deletions test/components/form/new.spec.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { clone } from 'ramda';

import Alert40917 from '../../../src/components/alert/40917.vue';
import ChecklistStep from '../../../src/components/checklist-step.vue';
import FileDropZone from '../../../src/components/file-drop-zone.vue';
import FormNew from '../../../src/components/form/new.vue';
@@ -501,10 +502,10 @@ describe('FormNew', () => {
details: { duplicateProperties: [{ current: 'first_name', provided: 'FIRST_NAME' }] }
})
.afterResponse(modal => {
modal.should.alert(
'danger',
/This Form attempts to create a new Entity property that matches with an existing one except for capitalization:.*FIRST_NAME \(existing: first_name\)/s
);
modal.should.alert('danger', [
Alert40917,
{ duplicateProperties: [{ current: 'first_name', provided: 'FIRST_NAME' }] }
]);
});
});
});