Merge pull request #40014 from nextcloud/fix/37899-The_apps_table_rep…

…resents_a_table_element_and_should_be_implemented_as_such._ Create right table view
nextcloud · Sep 6, 2023 · 00200a2 · 00200a2
2 parents 9df4da8 + 8b60295
commit 00200a2
Show file tree

Hide file tree

Showing 10 changed files with 125 additions and 54 deletions.
diff --git a/apps/settings/css/settings.css b/apps/settings/css/settings.css
diff --git a/apps/settings/css/settings.css.map b/apps/settings/css/settings.css.map
diff --git a/apps/settings/css/settings.scss b/apps/settings/css/settings.scss
@@ -842,10 +842,6 @@ span.version {
 	overflow: hidden;
 }
 
-.app-name, .app-version, .app-score, .app-level {
-	display: inline-block;
-}
-
 .app-description-toggle-show, .app-description-toggle-hide {
 	clear: both;
 	padding: 7px 0;
@@ -918,7 +914,6 @@ span.version {
 			display: table;
 			width: 100%;
 			height: auto;
-			margin-top: $toolbar-height;
 		}
 
 		margin-bottom: 100px;
@@ -1042,7 +1037,6 @@ span.version {
 
 	/* Bundle header */
 	.apps-header {
-		display: table-row;
 		position: relative;
 
 		div {
@@ -1051,11 +1045,9 @@ span.version {
 		}
 
 		h2 {
-			display: table-cell;
-			position: absolute;
 			padding-left: 6px;
 			padding-top: 15px;
-
+			margin-bottom: 12px;
 			.enable {
 				position: relative;
 				top: -1px;

diff --git a/apps/settings/src/components/AppList.vue b/apps/settings/src/components/AppList.vue
@@ -38,35 +38,71 @@
 					{{ t('settings', 'All apps are up-to-date.') }}
 				</div>
 
-				<transition-group name="app-list" tag="div" class="apps-list-container">
+				<transition-group name="app-list" tag="table" class="apps-list-container">
+					<tr key="app-list-view-header" class="apps-header">
+						<th class="app-image">
+							<span class="hidden-visually">{{ t('settings', 'Icon') }}</span>
+						</th>
+						<th class="app-name">
+							<span class="hidden-visually">{{ t('settings', 'Name') }}</span>
+						</th>
+						<th class="app-version">
+							<span class="hidden-visually">{{ t('settings', 'Version') }}</span>
+						</th>
+						<th class="app-level">
+							<span class="hidden-visually">{{ t('settings', 'Level') }}</span>
+						</th>
+						<th class="actions">
+							<span class="hidden-visually">{{ t('settings', 'Actions') }}</span>
+						</th>
+					</tr>
 					<AppItem v-for="app in apps"
 						:key="app.id"
 						:app="app"
 						:category="category" />
 				</transition-group>
 			</template>
 
-			<transition-group v-if="useBundleView"
-				name="app-list"
-				tag="div"
+			<table v-if="useBundleView"
 				class="apps-list-container">
+				<tr key="app-list-view-header" class="apps-header">
+					<th id="app-table-col-icon" class="app-image">
+						<span class="hidden-visually">{{ t('settings', 'Icon') }}</span>
+					</th>
+					<th id="app-table-col-name" class="app-name">
+						<span class="hidden-visually">{{ t('settings', 'Name') }}</span>
+					</th>
+					<th id="app-table-col-version" class="app-version">
+						<span class="hidden-visually">{{ t('settings', 'Version') }}</span>
+					</th>
+					<th id="app-table-col-level" class="app-level">
+						<span class="hidden-visually">{{ t('settings', 'Level') }}</span>
+					</th>
+					<th id="app-table-col-actions" class="actions">
+						<span class="hidden-visually">{{ t('settings', 'Actions') }}</span>
+					</th>
+				</tr>
 				<template v-for="bundle in bundles">
-					<div :key="bundle.id" class="apps-header">
-						<div class="app-image" />
-						<h2>{{ bundle.name }} <input type="button" :value="bundleToggleText(bundle.id)" @click="toggleBundle(bundle.id)"></h2>
-						<div class="app-version" />
-						<div class="app-level" />
-						<div class="app-groups" />
-						<div class="actions">
-							&nbsp;
-						</div>
-					</div>
+					<tr :key="bundle.id">
+						<th :id="`app-table-rowgroup-${bundle.id}`" colspan="5" scope="rowgroup">
+							<div class="app-bundle-heading">
+								<span class="app-bundle-header">
+									{{ bundle.name }}
+								</span>
+								<NcButton type="secondary" @click="toggleBundle(bundle.id)">
+									{{ t('settings', bundleToggleText(bundle.id)) }}
+								</NcButton>
+							</div>
+						</th>
+					</tr>
 					<AppItem v-for="app in bundleApps(bundle.id)"
 						:key="bundle.id + app.id"
+						:use-bundle-view="true"
+						:headers="`app-table-rowgroup-${bundle.id}`"
 						:app="app"
 						:category="category" />
 				</template>
-			</transition-group>
+			</table>
 			<template v-if="useAppStoreView">
 				<AppItem v-for="app in apps"
 					:key="app.id"
@@ -88,8 +124,7 @@
 					<AppItem v-for="app in searchApps"
 						:key="app.id"
 						:app="app"
-						:category="category"
-						:list-view="true" />
+						:category="category" />
 				</template>
 			</div>
 		</div>
@@ -240,9 +275,24 @@ export default {
 			const limit = pLimit(1)
 			this.apps
 				.filter(app => app.update)
-				.map(app => limit(() => this.$store.dispatch('updateApp', { appId: app.id }))
+				.map(app => limit(() => this.$store.dispatch('updateApp', { appId: app.id })),
 				)
 		},
 	},
 }
 </script>
+
+<style lang="scss" scoped>
+	.app-bundle-heading {
+		display: flex;
+		align-items: center;
+		margin: 20px 10px 20px 0;
+	}
+	.app-bundle-header {
+		margin: 0 10px 0 50px;
+		font-weight: bold;
+		font-size: 20px;
+		line-height: 30px;
+		color: var(--color-text-light);
+	}
+</style>
diff --git a/apps/settings/src/components/AppList/AppItem.vue b/apps/settings/src/components/AppList/AppItem.vue
@@ -21,8 +21,14 @@
   -->
 
 <template>
-	<div class="section" :class="{ selected: isSelected }" @click="showAppDetails">
-		<div class="app-image app-image-icon" @click="showAppDetails">
+	<component :is="listView ? `tr` : `div`"
+		class="section"
+		:class="{ selected: isSelected }"
+		@click="showAppDetails">
+		<component :is="dataItemTag"
+			class="app-image app-image-icon"
+			:headers="getDataItemHeaders(`app-table-col-icon`)"
+			@click="showAppDetails">
 			<div v-if="(listView && !app.preview) || (!listView && !screenshotLoaded)" class="icon-settings-dark" />
 
 			<svg v-else-if="listView && app.preview"
@@ -39,19 +45,28 @@
 			</svg>
 
 			<img v-if="!listView && app.screenshot && screenshotLoaded" :src="app.screenshot" width="100%">
-		</div>
-		<div class="app-name" @click="showAppDetails">
+		</component>
+		<component :is="dataItemTag"
+			class="app-name"
+			:headers="getDataItemHeaders(`app-table-col-name`)"
+			@click="showAppDetails">
 			{{ app.name }}
-		</div>
-		<div v-if="!listView" class="app-summary">
+		</component>
+		<component :is="dataItemTag"
+			v-if="!listView"
+			class="app-summary"
+			:headers="getDataItemHeaders(`app-version`)">
 			{{ app.summary }}
-		</div>
-		<div v-if="listView" class="app-version">
+		</component>
+		<component :is="dataItemTag"
+			v-if="listView"
+			class="app-version"
+			:headers="getDataItemHeaders(`app-table-col-version`)">
 			<span v-if="app.version">{{ app.version }}</span>
 			<span v-else-if="app.appstoreData.releases[0].version">{{ app.appstoreData.releases[0].version }}</span>
-		</div>
+		</component>
 
-		<div class="app-level">
+		<component :is="dataItemTag" :headers="getDataItemHeaders(`app-table-col-level`)" class="app-level">
 			<span v-if="app.level === 300"
 				:title="t('settings', 'This app is supported via your current Nextcloud subscription.')"
 				:aria-label="t('settings', 'This app is supported via your current Nextcloud subscription.')"
@@ -63,9 +78,8 @@
 				class="official icon-checkmark">
 				{{ t('settings', 'Featured') }}</span>
 			<AppScore v-if="hasRating && !listView" :score="app.score" />
-		</div>
-
-		<div class="actions">
+		</component>
+		<component :is="dataItemTag" :headers="getDataItemHeaders(`app-table-col-actions`)" class="actions">
 			<div v-if="app.error" class="warning">
 				{{ app.error }}
 			</div>
@@ -104,8 +118,8 @@
 				@click.stop="forceEnable(app.id)">
 				{{ forceEnableButtonText }}
 			</NcButton>
-		</div>
-	</div>
+		</component>
+	</component>
 </template>
 
 <script>
@@ -128,6 +142,14 @@ export default {
 			type: Boolean,
 			default: true,
 		},
+		useBundleView: {
+			type: Boolean,
+			default: false,
+		},
+		headers: {
+			type: String,
+			default: null,
+		},
 	},
 	data() {
 		return {
@@ -140,6 +162,9 @@ export default {
 		hasRating() {
 			return this.app.appstoreData && this.app.appstoreData.ratingNumOverall > 5
 		},
+		dataItemTag() {
+			return this.listView ? 'td' : 'div'
+		},
 	},
 	watch: {
 		'$route.params.id'(id) {
@@ -176,6 +201,10 @@ export default {
 		prefix(prefix, content) {
 			return prefix + '_' + content
 		},
+
+		getDataItemHeaders(columnName) {
+			return this.useBundleView ? [this.headers, columnName].join(' ') : null
+		},
 	},
 }
 </script>

diff --git a/dist/settings-apps-view-7418.js b/dist/settings-apps-view-7418.js
diff --git a/dist/settings-apps-view-7418.js.map b/dist/settings-apps-view-7418.js.map
diff --git a/dist/settings-vue-settings-apps-users-management.js b/dist/settings-vue-settings-apps-users-management.js
diff --git a/dist/settings-vue-settings-apps-users-management.js.map b/dist/settings-vue-settings-apps-users-management.js.map
diff --git a/tests/acceptance/features/bootstrap/AppsManagementContext.php b/tests/acceptance/features/bootstrap/AppsManagementContext.php
@@ -33,7 +33,7 @@ class AppsManagementContext implements Context, ActorAwareInterface {
 	 * @return Locator
 	 */
 	public static function appsList() {
-		return Locator::forThe()->xpath("//main[@id='app-content' or contains(@class, 'app-content')]//div[@id='apps-list']")->
+		return Locator::forThe()->xpath("//main[@id='app-content' or contains(@class, 'app-content')]//*[@id='apps-list']")->
 				describedAs("Apps list in Apps Management");
 	}
 
@@ -86,7 +86,7 @@ public static function disableButtonForAnyApp() {
 	 * @return Locator
 	 */
 	public static function enableAllBundleButton($bundle) {
-		return Locator::forThe()->xpath("//div[@class='apps-header']/h2[normalize-space() = '$bundle']/input[@value='Download and enable all']")->
+		return Locator::forThe()->xpath("//th[//*[normalize-space() = '$bundle']]//button[normalize-space() = 'Download and enable all']")->
 				descendantOf(self::appsList())->
 				describedAs("Button to enable bundles");
 	}
@@ -95,7 +95,7 @@ public static function enableAllBundleButton($bundle) {
 	 * @return Locator
 	 */
 	public static function rowForApp($app) {
-		return Locator::forThe()->xpath("//div[@class='app-name'][normalize-space() = '$app']/..")->
+		return Locator::forThe()->xpath("//*[@class='app-name'][normalize-space() = '$app']/..")->
 				descendantOf(self::appsList())->
 				describedAs("Row for app $app in Apps Management");
 	}
@@ -104,7 +104,7 @@ public static function rowForApp($app) {
 	 * @return Locator
 	 */
 	public static function emptyAppList() {
-		return Locator::forThe()->xpath("//div[@id='apps-list-empty']")->
+		return Locator::forThe()->xpath("//*[@id='apps-list-empty']")->
 				descendantOf(self::appsList())->
 				describedAs("Empty apps list view");
 	}