Refine input validation

Signed-off-by: Christopher Ng <chrng8@gmail.com>
nextcloud · Aug 24, 2021 · cdf59cb · cdf59cb
1 parent b4203be
commit cdf59cb
Show file tree

Hide file tree

Showing 8 changed files with 89 additions and 79 deletions.
diff --git a/apps/settings/src/components/PersonalInfo/DisplayNameSection/DisplayName.vue b/apps/settings/src/components/PersonalInfo/DisplayNameSection/DisplayName.vue
@@ -128,19 +128,19 @@ export default {
 	display: grid;
 	align-items: center;
 
-		input {
-			grid-area: 1 / 1;
-			height: 34px;
-			width: 100%;
-			margin: 3px 3px 3px 0;
-			padding: 7px 6px;
-			cursor: text;
-			font-family: var(--font-face);
-			border: 1px solid var(--color-border-dark);
-			border-radius: var(--border-radius);
-			background-color: var(--color-main-background);
-			color: var(--color-main-text);
-		}
+	input {
+		grid-area: 1 / 1;
+		width: 100%;
+		height: 34px;
+		margin: 3px 3px 3px 0;
+		padding: 7px 6px;
+		color: var(--color-main-text);
+		border: 1px solid var(--color-border-dark);
+		border-radius: var(--border-radius);
+		background-color: var(--color-main-background);
+		font-family: var(--font-face);
+		cursor: text;
+	}
 
 	.displayname__actions-container {
 		grid-area: 1 / 1;

diff --git a/apps/settings/src/components/PersonalInfo/DisplayNameSection/DisplayNameSection.vue b/apps/settings/src/components/PersonalInfo/DisplayNameSection/DisplayNameSection.vue
@@ -80,12 +80,11 @@ export default {
 </script>
 
 <style lang="scss" scoped>
-	section {
-		padding: 10px 10px;
+section {
+	padding: 10px 10px;
 
-		&::v-deep button:disabled {
-			cursor: default;
-		}
+	&::v-deep button:disabled {
+		cursor: default;
 	}
 }
 </style>
diff --git a/apps/settings/src/components/PersonalInfo/EmailSection/Email.vue b/apps/settings/src/components/PersonalInfo/EmailSection/Email.vue
@@ -302,24 +302,19 @@ export default {
 	display: grid;
 	align-items: center;
 
-		input {
-			grid-area: 1 / 1;
-			height: 34px;
-			width: 100%;
-			margin: 3px 3px 3px 0;
-			padding: 7px 6px;
-			cursor: text;
-			font-family: var(--font-face);
-			border: 1px solid var(--color-border-dark);
-			border-radius: var(--border-radius);
-			background-color: var(--color-main-background);
-			color: var(--color-main-text);
-		}
-
-		.email__actions-container {
-			grid-area: 1 / 1;
-			justify-self: flex-end;
-			height: 30px;
+	input {
+		grid-area: 1 / 1;
+		width: 100%;
+		height: 34px;
+		margin: 3px 3px 3px 0;
+		padding: 7px 6px;
+		color: var(--color-main-text);
+		border: 1px solid var(--color-border-dark);
+		border-radius: var(--border-radius);
+		background-color: var(--color-main-background);
+		font-family: var(--font-face);
+		cursor: text;
+	}
 
 	.email__actions-container {
 		grid-area: 1 / 1;

diff --git a/apps/settings/src/components/PersonalInfo/EmailSection/EmailSection.vue b/apps/settings/src/components/PersonalInfo/EmailSection/EmailSection.vue
@@ -164,12 +164,11 @@ export default {
 </script>
 
 <style lang="scss" scoped>
-	section {
-		padding: 10px 10px;
+section {
+	padding: 10px 10px;
 
-		&::v-deep button:disabled {
-			cursor: default;
-		}
+	&::v-deep button:disabled {
+		cursor: default;
 	}
 }
 </style>
diff --git a/apps/settings/src/components/PersonalInfo/LanguageSection/Language.vue b/apps/settings/src/components/PersonalInfo/LanguageSection/Language.vue
@@ -27,7 +27,7 @@
 			name="language"
 			:placeholder="t('settings', 'Language')"
 			required
-			@input="onLanguageChange">
+			@change="onLanguageChange">
 			<option v-for="commonLanguage in commonLanguages"
 				:key="commonLanguage.code"
 				:selected="language.code === commonLanguage.code"
@@ -58,6 +58,7 @@
 import { showError } from '@nextcloud/dialogs'
 
 import { saveLanguage } from '../../../service/PersonalInfo/LanguageService'
+import { validateLanguage } from '../../../utils/validate'
 
 export default {
 	name: 'Language',
@@ -97,7 +98,7 @@ export default {
 			const language = this.constructLanguage(e.target.value)
 			this.$emit('update:language', language)
 
-			if (this.$refs.language?.checkValidity()) {
+			if (validateLanguage(language)) {
 				await this.updateLanguage(language)
 			}
 		},
@@ -146,7 +147,23 @@ export default {
 .language {
 	display: grid;
 
+	select {
+		width: 100%;
+		height: 34px;
+		margin: 3px 3px 3px 0;
+		padding: 6px 16px;
+		color: var(--color-main-text);
+		border: 1px solid var(--color-border-dark);
+		border-radius: var(--border-radius);
+		background: var(--icon-triangle-s-000) no-repeat right 4px center;
+		font-family: var(--font-face);
+		appearance: none;
+		cursor: pointer;
+	}
+
 	a {
+		color: var(--color-main-text);
+		text-decoration: none;
 		width: max-content;
 	}
 }

diff --git a/apps/settings/src/components/PersonalInfo/LanguageSection/LanguageSection.vue b/apps/settings/src/components/PersonalInfo/LanguageSection/LanguageSection.vue
@@ -20,27 +20,23 @@
 -->
 
 <template>
-	<form
-		ref="form"
-		class="section"
-		@submit.stop.prevent="() => {}">
+	<section>
 		<HeaderBar
 			:account-property="accountProperty"
 			label-for="language"
-			:is-valid-form="isValidForm" />
+			:is-valid-section="isValidSection" />
 
 		<template v-if="isEditable">
 			<Language
 				:common-languages="commonLanguages"
 				:other-languages="otherLanguages"
-				:language.sync="language"
-				@update:language="onUpdateLanguage" />
+				:language.sync="language" />
 		</template>
 
 		<span v-else>
 			{{ t('settings', 'No language set') }}
 		</span>
-	</form>
+	</section>
 </template>
 
 <script>
@@ -50,6 +46,7 @@ import Language from './Language'
 import HeaderBar from '../shared/HeaderBar'
 
 import { SETTING_PROPERTY_READABLE_ENUM } from '../../../constants/AccountPropertyConstants'
+import { validateLanguage } from '../../../utils/validate'
 
 const { languages: { activeLanguage, commonLanguages, otherLanguages } } = loadState('settings', 'personalInfoParameters', {})
 
@@ -64,7 +61,6 @@ export default {
 	data() {
 		return {
 			accountProperty: SETTING_PROPERTY_READABLE_ENUM.LANGUAGE,
-			isValidForm: true,
 			commonLanguages,
 			otherLanguages,
 			language: activeLanguage,
@@ -75,27 +71,19 @@ export default {
 		isEditable() {
 			return Boolean(this.language)
 		},
-	},
-
-	mounted() {
-		this.$nextTick(() => this.updateFormValidity())
-	},
 
-	methods: {
-		onUpdateLanguage() {
-			this.$nextTick(() => this.updateFormValidity())
-		},
-
-		updateFormValidity() {
-			this.isValidForm = this.$refs.form?.checkValidity()
+		isValidSection() {
+			return validateLanguage(this.language)
 		},
 	},
 }
 </script>
 
 <style lang="scss" scoped>
-form::v-deep button {
-	&:disabled {
+section {
+	padding: 10px 10px;
+
+	&::v-deep button:disabled {
 		cursor: default;
 	}
 }

diff --git a/apps/settings/src/components/PersonalInfo/shared/HeaderBar.vue b/apps/settings/src/components/PersonalInfo/shared/HeaderBar.vue
@@ -117,26 +117,21 @@ export default {
 
 <style lang="scss" scoped>
 	h3 {
-    display: inline-flex;
-    width: 100%;
-    margin: 12px 0 0 0;
+		display: inline-flex;
+		width: 100%;
+		margin: 12px 0 0 0;
 		font-size: 16px;
-    color: var(--color-text-light);
+		color: var(--color-text-light);
+
+		&.setting-property {
+			height: 38px;
+		}
 
 		label {
 			cursor: pointer;
 		}
 	}
 
-	h3.setting-property {
-    width: 100%;
-		min-height: 38px;
-    display: inline-flex;
-		position: relative;
-    flex-wrap: nowrap;
-    justify-content: flex-start;
-	}
-
 	.federation-control {
 		margin: -12px 0 0 8px;
 	}

diff --git a/apps/settings/src/utils/validate.js b/apps/settings/src/utils/validate.js
@@ -20,6 +20,12 @@
  *
  */
 
+/*
+ * Frontend validators, less strict than backend validators
+ *
+ * TODO add nice validation errors for Profile page settings modal
+ */
+
 import { VALIDATE_EMAIL_REGEX } from '../constants/AccountPropertyConstants'
 
 /**
@@ -49,3 +55,14 @@ export function validateEmail(input) {
 		&& input.length <= 320
 		&& encodeURIComponent(input).replace(/%../g, 'x').length <= 320
 }
+
+/**
+ * Validate the language input
+ *
+ * @param {string} input the input
+ * @returns {boolean}
+ */
+export function validateLanguage(input) {
+	return input.code !== ''
+		&& input.name
+}