Skip to content

Commit e9ccfbe

Browse files
PytalPytal
committed
Refine input validation
- Remove usage of JS core checkValidity() in favour of custom backend compliant validation - Rewrite and refactor with removal of form tag in favour of section - Scope styles - Remove many uses of $nextTick - Refine disabled state logic - Translate account property constants Signed-off-by: Christopher Ng <chrng8@gmail.com>
1 parent 5e67677 commit e9ccfbe

8 files changed

Lines changed: 135 additions & 82 deletions

File tree

‎apps/settings/src/components/PersonalInfo/DisplayNameSection/DisplayName.vue‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ import { showError } from '@nextcloud/dialogs'
4848
import debounce from 'debounce'
4949
5050
import { savePrimaryDisplayName } from '../../../service/PersonalInfo/DisplayNameService'
51+
import { validateDisplayName } from '../../../utils/validate'
5152
5253
// TODO Global avatar updating on events (e.g. updating the displayname) is currently being handled by global js, investigate using https://github.com/nextcloud/nextcloud-event-bus for global avatar updating
5354
@@ -81,7 +82,7 @@ export default {
8182
},
8283
8384
debounceDisplayNameChange: debounce(async function(displayName) {
84-
if (this.$refs.displayName?.checkValidity() && this.isValid(displayName)) {
85+
if (validateDisplayName(displayName)) {
8586
await this.updatePrimaryDisplayName(displayName)
8687
}
8788
}, 500),
@@ -115,10 +116,6 @@ export default {
115116
}
116117
},
117118
118-
isValid(displayName) {
119-
return displayName !== ''
120-
},
121-
122119
onScopeChange(scope) {
123120
this.$emit('update:scope', scope)
124121
},
@@ -131,8 +128,18 @@ export default {
131128
display: grid;
132129
align-items: center;
133130
134-
input[type=text] {
131+
input {
135132
grid-area: 1 / 1;
133+
height: 34px;
134+
width: 100%;
135+
margin: 3px 3px 3px 0;
136+
padding: 7px 6px;
137+
cursor: text;
138+
font-family: var(--font-face);
139+
border: 1px solid var(--color-border-dark);
140+
border-radius: var(--border-radius);
141+
background-color: var(--color-main-background);
142+
color: var(--color-main-text);
136143
}
137144
138145
.displayname__actions-container {

‎apps/settings/src/components/PersonalInfo/DisplayNameSection/DisplayNameSection.vue‎

Lines changed: 12 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -20,29 +20,25 @@
2020
-->
2121

2222
<template>
23-
<form
24-
ref="form"
25-
class="section"
26-
@submit.stop.prevent="() => {}">
23+
<section>
2724
<HeaderBar
2825
:account-property="accountProperty"
2926
label-for="displayname"
3027
:is-editable="displayNameChangeSupported"
31-
:is-valid-form="isValidForm"
28+
:is-valid-section="isValidSection"
3229
:handle-scope-change="savePrimaryDisplayNameScope"
3330
:scope.sync="primaryDisplayName.scope" />
3431

3532
<template v-if="displayNameChangeSupported">
3633
<DisplayName
37-
:scope.sync="primaryDisplayName.scope"
3834
:display-name.sync="primaryDisplayName.value"
39-
@update:display-name="onUpdateDisplayName" />
35+
:scope.sync="primaryDisplayName.scope" />
4036
</template>
4137

4238
<span v-else>
4339
{{ primaryDisplayName.value || t('settings', 'No full name set') }}
4440
</span>
45-
</form>
41+
</section>
4642
</template>
4743

4844
<script>
@@ -53,6 +49,7 @@ import HeaderBar from '../shared/HeaderBar'
5349
5450
import { ACCOUNT_PROPERTY_READABLE_ENUM } from '../../../constants/AccountPropertyConstants'
5551
import { savePrimaryDisplayNameScope } from '../../../service/PersonalInfo/DisplayNameService'
52+
import { validateDisplayName } from '../../../utils/validate'
5653
5754
const { displayNames: { primaryDisplayName } } = loadState('settings', 'personalInfoParameters', {})
5855
const { displayNameChangeSupported } = loadState('settings', 'accountParameters', {})
@@ -69,31 +66,24 @@ export default {
6966
return {
7067
accountProperty: ACCOUNT_PROPERTY_READABLE_ENUM.DISPLAYNAME,
7168
displayNameChangeSupported,
72-
isValidForm: true,
7369
primaryDisplayName,
7470
savePrimaryDisplayNameScope,
7571
}
7672
},
7773
78-
mounted() {
79-
this.$nextTick(() => this.updateFormValidity())
80-
},
81-
82-
methods: {
83-
onUpdateDisplayName() {
84-
this.$nextTick(() => this.updateFormValidity())
85-
},
86-
87-
updateFormValidity() {
88-
this.isValidForm = this.$refs.form?.checkValidity()
74+
computed: {
75+
isValidSection() {
76+
return validateDisplayName(this.primaryDisplayName.value)
8977
},
9078
},
9179
}
9280
</script>
9381

9482
<style lang="scss" scoped>
95-
form::v-deep button {
96-
&:disabled {
83+
section {
84+
padding: 10px 10px;
85+
86+
&::v-deep button:disabled {
9787
cursor: default;
9888
}
9989
}

‎apps/settings/src/components/PersonalInfo/EmailSection/Email.vue‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@
5959
<ActionButton
6060
:aria-label="deleteEmailLabel"
6161
:close-after-click="true"
62+
:disabled="deleteDisabled"
6263
icon="icon-delete"
6364
@click.stop.prevent="deleteEmail">
6465
{{ deleteEmailLabel }}
@@ -83,6 +84,7 @@ import FederationControl from '../shared/FederationControl'
8384
8485
import { ACCOUNT_PROPERTY_READABLE_ENUM } from '../../../constants/AccountPropertyConstants'
8586
import { savePrimaryEmail, saveAdditionalEmail, saveAdditionalEmailScope, updateAdditionalEmail, removeAdditionalEmail } from '../../../service/PersonalInfo/EmailService'
87+
import { validateEmail } from '../../../utils/validate'
8688
8789
export default {
8890
name: 'Email',
@@ -126,9 +128,13 @@ export default {
126128
computed: {
127129
deleteDisabled() {
128130
if (this.primary) {
129-
return this.email === ''
131+
// Disable for empty primary email as there is nothing to delete
132+
// OR when initialEmail (reflects server state) and email (current input) are not the same
133+
return this.email === '' || this.initialEmail !== this.email
134+
} else if (this.initialEmail !== '') {
135+
return this.initialEmail !== this.email
130136
}
131-
return this.email !== '' && !this.isValid(this.email)
137+
return false
132138
},
133139
134140
deleteEmailLabel() {
@@ -159,6 +165,7 @@ export default {
159165
160166
mounted() {
161167
if (!this.primary && this.initialEmail === '') {
168+
// $nextTick is needed here, otherwise it may not always work https://stackoverflow.com/questions/51922767/autofocus-input-on-mount-vue-ios/63485725#63485725
162169
this.$nextTick(() => this.$refs.email?.focus())
163170
}
164171
},
@@ -170,7 +177,7 @@ export default {
170177
},
171178
172179
debounceEmailChange: debounce(async function(email) {
173-
if (this.$refs.email?.checkValidity() || email === '') {
180+
if (validateEmail(email) || email === '') {
174181
if (this.primary) {
175182
await this.updatePrimaryEmail(email)
176183
} else {
@@ -282,10 +289,6 @@ export default {
282289
}
283290
},
284291
285-
isValid(email) {
286-
return /^\S+$/.test(email)
287-
},
288-
289292
onScopeChange(scope) {
290293
this.$emit('update:scope', scope)
291294
},
@@ -298,8 +301,18 @@ export default {
298301
display: grid;
299302
align-items: center;
300303
301-
input[type=email] {
304+
input {
302305
grid-area: 1 / 1;
306+
height: 34px;
307+
width: 100%;
308+
margin: 3px 3px 3px 0;
309+
padding: 7px 6px;
310+
cursor: text;
311+
font-family: var(--font-face);
312+
border: 1px solid var(--color-border-dark);
313+
border-radius: var(--border-radius);
314+
background-color: var(--color-main-background);
315+
color: var(--color-main-text);
303316
}
304317
305318
.email__actions-container {

‎apps/settings/src/components/PersonalInfo/EmailSection/EmailSection.vue‎

Lines changed: 21 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -20,17 +20,14 @@
2020
-->
2121

2222
<template>
23-
<form
24-
ref="form"
25-
class="section"
26-
@submit.stop.prevent="() => {}">
23+
<section>
2724
<HeaderBar
2825
:account-property="accountProperty"
2926
label-for="email"
3027
:handle-scope-change="savePrimaryEmailScope"
3128
:is-editable="displayNameChangeSupported"
3229
:is-multi-value-supported="true"
33-
:is-valid-form="isValidForm"
30+
:is-valid-section="isValidSection"
3431
:scope.sync="primaryEmail.scope"
3532
@add-additional="onAddAdditionalEmail" />
3633

@@ -52,7 +49,7 @@
5249
<span v-else>
5350
{{ primaryEmail.value || t('settings', 'No email address set') }}
5451
</span>
55-
</form>
52+
</section>
5653
</template>
5754

5855
<script>
@@ -64,6 +61,7 @@ import HeaderBar from '../shared/HeaderBar'
6461
6562
import { ACCOUNT_PROPERTY_READABLE_ENUM, DEFAULT_ADDITIONAL_EMAIL_SCOPE } from '../../../constants/AccountPropertyConstants'
6663
import { savePrimaryEmail, savePrimaryEmailScope, removeAdditionalEmail } from '../../../service/PersonalInfo/EmailService'
64+
import { validateEmail } from '../../../utils/validate'
6765
6866
const { emails: { additionalEmails, primaryEmail } } = loadState('settings', 'personalInfoParameters', {})
6967
const { displayNameChangeSupported } = loadState('settings', 'accountParameters', {})
@@ -81,13 +79,24 @@ export default {
8179
accountProperty: ACCOUNT_PROPERTY_READABLE_ENUM.EMAIL,
8280
additionalEmails,
8381
displayNameChangeSupported,
84-
isValidForm: true,
8582
primaryEmail,
8683
savePrimaryEmailScope,
8784
}
8885
},
8986
9087
computed: {
88+
firstAdditionalEmail() {
89+
if (this.additionalEmails.length) {
90+
return this.additionalEmails[0].value
91+
}
92+
return null
93+
},
94+
95+
isValidSection() {
96+
return validateEmail(this.primaryEmail.value)
97+
&& this.additionalEmails.map(({ value }) => value).every(validateEmail)
98+
},
99+
91100
primaryEmailValue: {
92101
get() {
93102
return this.primaryEmail.value
@@ -96,41 +105,25 @@ export default {
96105
this.primaryEmail.value = value
97106
},
98107
},
99-
100-
firstAdditionalEmail() {
101-
if (this.additionalEmails.length) {
102-
return this.additionalEmails[0].value
103-
}
104-
return null
105-
},
106-
},
107-
108-
mounted() {
109-
this.$nextTick(() => this.updateFormValidity())
110108
},
111109
112110
methods: {
113111
onAddAdditionalEmail() {
114-
if (this.$refs.form?.checkValidity()) {
112+
if (this.isValidSection) {
115113
this.additionalEmails.push({ value: '', scope: DEFAULT_ADDITIONAL_EMAIL_SCOPE })
116-
this.$nextTick(() => this.updateFormValidity())
117114
}
118115
},
119116
120117
onDeleteAdditionalEmail(index) {
121118
this.$delete(this.additionalEmails, index)
122-
this.$nextTick(() => this.updateFormValidity())
123119
},
124120
125121
async onUpdateEmail() {
126-
this.$nextTick(() => this.updateFormValidity())
127-
128122
if (this.primaryEmailValue === '' && this.firstAdditionalEmail) {
129123
const deletedEmail = this.firstAdditionalEmail
130124
await this.deleteFirstAdditionalEmail()
131125
this.primaryEmailValue = deletedEmail
132126
await this.updatePrimaryEmail()
133-
this.$nextTick(() => this.updateFormValidity())
134127
}
135128
},
136129
@@ -166,17 +159,15 @@ export default {
166159
this.logger.error(errorMessage, error)
167160
}
168161
},
169-
170-
updateFormValidity() {
171-
this.isValidForm = this.$refs.form?.checkValidity()
172-
},
173162
},
174163
}
175164
</script>
176165
177166
<style lang="scss" scoped>
178-
form::v-deep button {
179-
&:disabled {
167+
section {
168+
padding: 10px 10px;
169+
170+
&::v-deep button:disabled {
180171
cursor: default;
181172
}
182173
}

‎apps/settings/src/components/PersonalInfo/shared/FederationControl.vue‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ export default {
8787
8888
data() {
8989
return {
90-
accountPropertyLowerCase: this.accountProperty.toLowerCase(),
90+
accountPropertyLowerCase: this.accountProperty.toLocaleLowerCase(),
9191
initialScope: this.scope,
9292
}
9393
},

‎apps/settings/src/components/PersonalInfo/shared/HeaderBar.vue‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@
2222
<template>
2323
<h3>
2424
<label :for="labelFor">
25-
{{ t('settings', accountProperty) }}
25+
<!-- Already translated as required by prop validator -->
26+
{{ accountProperty }}
2627
</label>
2728

2829
<FederationControl
@@ -35,7 +36,7 @@
3536
<template v-if="isEditable && isMultiValueSupported">
3637
<AddButton
3738
class="add-button"
38-
:disabled="!isValidForm"
39+
:disabled="!isValidSection"
3940
@click.stop.prevent="onAddAdditional" />
4041
</template>
4142
</h3>
@@ -73,7 +74,7 @@ export default {
7374
type: Boolean,
7475
default: false,
7576
},
76-
isValidForm: {
77+
isValidSection: {
7778
type: Boolean,
7879
default: true,
7980
},
@@ -106,6 +107,18 @@ export default {
106107
</script>
107108

108109
<style lang="scss" scoped>
110+
h3 {
111+
display: inline-flex;
112+
width: 100%;
113+
margin: 12px 0 0 0;
114+
font-size: 16px;
115+
color: var(--color-text-light);
116+
117+
label {
118+
cursor: pointer;
119+
}
120+
}
121+
109122
.federation-control {
110123
margin: -12px 0 0 8px;
111124
}

0 commit comments

Comments
 (0)