Enh/beta improvements #41481

marcoambrosini · 2023-11-15T05:51:52Z

Resolves: Beta design feedback #41450

marcoambrosini · 2023-11-15T05:52:34Z

There's a problem with the starring method, it does not work. Nor in master nor in this pr. Investigating the issue
@nextcloud/server-frontend any ideas?

marcoambrosini · 2023-11-15T05:59:56Z

Those unquoted symbols don't look right @jancborchardt

Making the input and comments full width, solves the issue too and tidies up things a lot:

Before	After

jancborchardt

The favorite indicator needs to show left in the subline still, no? Or is this related to your comment about the favoriting not working?
I think the unquoted symbols are fine, the quotes are much too busy. If possible the symbols could be color-main-text (not bold) to make them stand out a bit.
Not keeping a left gap for the avatars (and not indenting the messages) just looks off and too busy. If you look at Discord mobile for example, they also habe a gap on the left. Additionally in Talk, we sticky the avatar so on long messages you still have the context, which is great. So we should keep that. As discussed, the issue in Talk is more the space reserved for the timestamp on the right. So let's look at that separately.

marcoambrosini · 2023-11-15T09:22:58Z

favorite indicator needs to show left in the subline still, no

yes, done!

szaimen · 2023-11-15T09:24:12Z

Do we display the favorited state now anywhere in the sidebar? If not, would that be a regression?

marcoambrosini · 2023-11-15T09:27:56Z

Yes @szaimen, in the subline

szaimen · 2023-11-15T09:30:09Z

Yes @szaimen, in the subline

ah nice! Then looks good to me from the screenshots 👍

susnux · 2023-11-15T11:38:54Z

apps/files/src/views/Sidebar.vue

@@ -246,7 +261,6 @@ export default {
 					},
 					compact: this.hasLowHeight || !this.fileInfo.hasPreview || this.isFullScreen,
 					loading: this.loading,
-					starred: this.fileInfo.isFavourited,


We should then decide if we simply want to drop this in the library as well.

susnux

LGTM

susnux · 2023-11-15T11:39:07Z

apps/files/src/views/Sidebar.vue

@@ -189,7 +203,8 @@ export default {
 		 * @return {string}
 		 */
 		subtitle() {
-			return `${this.size}, ${this.time}`
+			const starredIndicator = this.fileInfo.isFavourited ? '★ ' : ''


Suggested change

const starredIndicator = this.fileInfo.isFavourited ? '★ ' : ''

const starredIndicator = this.fileInfo.isFavourited ? '⭐ ' : ''

If we want to make it more discoverable / prominent that the file is a favorite we might want to use this instead.

Is this accessible? The grey one would meet the contrast requirements I would hope.

CC @JuliaKirschenheuter

I think we would have to use an icon with alt text / aria-label, right @JuliaKirschenheuter @marcoambrosini?

Well we have --icon-starred-yellow as an icon that fulfills accessibility requirements for bright and dark mode.

The problem is that the NcAppSidebar only accepts a string in that subline. I suggest we add the text "favorite" beside the star, so a screen reader would read out the info too.
Contrast is ok, yes.

Also not possible, there it's either the switch or nothing. I suggest we go ahead with this and we modify the sidebar component so that we can slot in a proper icon there as @susnux suggests

Also not possible, there it's either the switch or nothing.

Why is this not possible? Since the current solution is not accessible ("star" is not understandable as "favorite", and we can not add an aria-label) we need to do it otherwise.

Can we please do:

Just use an icon on the top right of the filetype icon, non-interactive like in the file list

The action to toggle it would be in the action menu, like you did here

@marcoambrosini @szaimen

Just use an icon on the top right of the filetype icon, non-interactive like in the file list

and we have already everything we need

Since the current solution is not accessible ("star" is not understandable as "favorite", and we can not add an aria-label) we need to do it otherwise.

I don't think that this is not accessible.

Emoji is read by a screen reader. It could be unclear that "star" means "favorite". But it is the same for all users. Any user needs to understand that "star" is "a starred file" which means "favorite".

This would be good to have a description here. But not required if we appeal to a common user experience of "starring".

Ok, thanks for the assessment @ShGKme – then @AndyScherzinger and I would agree we should merge this as-is for now.

In the long term however it could possibly be better to have an image with aria-label which is more explicit, which we could also place on the top right of the filetype icon so it is symmetric to the files list.

Signed-off-by: Marco <[email protected]>

AndyScherzinger · 2023-11-22T13:35:27Z

/compile amend /

Signed-off-by: Marco <[email protected]> Signed-off-by: nextcloud-command <[email protected]>

marcoambrosini force-pushed the enh/beta-improvements branch from f0b1f37 to 590e248 Compare November 15, 2023 05:54

marcoambrosini requested review from jancborchardt, a team, nfebe, sorbaugh, emoral435 and AndyScherzinger and removed request for a team November 15, 2023 05:54

marcoambrosini mentioned this pull request Nov 15, 2023

Beta design feedback #41450

Closed

13 tasks

marcoambrosini self-assigned this Nov 15, 2023

marcoambrosini added pending documentation This pull request needs an associated documentation update enhancement 2. developing Work in progress and removed pending documentation This pull request needs an associated documentation update labels Nov 15, 2023

jancborchardt reviewed Nov 15, 2023

View reviewed changes

szaimen approved these changes Nov 15, 2023

View reviewed changes

susnux reviewed Nov 15, 2023

View reviewed changes

susnux approved these changes Nov 15, 2023

View reviewed changes

solracsf added this to the Nextcloud 28 milestone Nov 21, 2023

blizzz mentioned this pull request Nov 22, 2023

28.0.0 RC1 #41621

Merged

5 tasks

jancborchardt approved these changes Nov 22, 2023

View reviewed changes

AndyScherzinger approved these changes Nov 22, 2023

View reviewed changes

marcoambrosini added 2 commits November 22, 2023 14:35

Move starring into actions menu

cd26b91

Signed-off-by: Marco <[email protected]>

Verticaly align comment avatar

eefbf6d

Signed-off-by: Marco <[email protected]>

Remove quotes from input

0da90ec

Signed-off-by: Marco <[email protected]>

AndyScherzinger force-pushed the enh/beta-improvements branch from a92b23a to 45cb11d Compare November 22, 2023 13:35

Add star indicator in file description

f476220

Signed-off-by: Marco <[email protected]> Signed-off-by: nextcloud-command <[email protected]>

nextcloud-command force-pushed the enh/beta-improvements branch from 45cb11d to f476220 Compare November 22, 2023 13:41

szaimen enabled auto-merge November 22, 2023 13:53

szaimen merged commit 46b0e1b into master Nov 22, 2023
41 checks passed

szaimen deleted the enh/beta-improvements branch November 22, 2023 15:29

susnux mentioned this pull request Nov 30, 2023

[BITV] 9.4.1.2/3.4 - The "Details" section contains a favorite button, where the status is only communicated to screen reader users in the "on" state using the aria-pressed="true" attribute. (2) #41892

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enh/beta improvements #41481

Enh/beta improvements #41481

marcoambrosini commented Nov 15, 2023 •

edited

Loading

marcoambrosini commented Nov 15, 2023 •

edited

Loading

marcoambrosini commented Nov 15, 2023 •

edited

Loading

jancborchardt left a comment

marcoambrosini commented Nov 15, 2023

szaimen commented Nov 15, 2023

marcoambrosini commented Nov 15, 2023 •

edited

Loading

szaimen commented Nov 15, 2023

susnux Nov 15, 2023

susnux left a comment

susnux Nov 15, 2023

AndyScherzinger Nov 15, 2023

jancborchardt Nov 15, 2023

susnux Nov 15, 2023

marcoambrosini Nov 16, 2023 •

edited

Loading

marcoambrosini Nov 17, 2023

jancborchardt Nov 22, 2023

JuliaKirschenheuter Nov 22, 2023

ShGKme Nov 22, 2023

jancborchardt Nov 22, 2023

AndyScherzinger commented Nov 22, 2023

	const starredIndicator = this.fileInfo.isFavourited ? '★ ' : ''
	const starredIndicator = this.fileInfo.isFavourited ? '⭐ ' : ''

Enh/beta improvements #41481

Enh/beta improvements #41481

Conversation

marcoambrosini commented Nov 15, 2023 • edited Loading

marcoambrosini commented Nov 15, 2023 • edited Loading

marcoambrosini commented Nov 15, 2023 • edited Loading

jancborchardt left a comment

Choose a reason for hiding this comment

marcoambrosini commented Nov 15, 2023

szaimen commented Nov 15, 2023

marcoambrosini commented Nov 15, 2023 • edited Loading

szaimen commented Nov 15, 2023

Choose a reason for hiding this comment

susnux left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcoambrosini Nov 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AndyScherzinger commented Nov 22, 2023

marcoambrosini commented Nov 15, 2023 •

edited

Loading

marcoambrosini commented Nov 15, 2023 •

edited

Loading

marcoambrosini commented Nov 15, 2023 •

edited

Loading

marcoambrosini commented Nov 15, 2023 •

edited

Loading

marcoambrosini Nov 16, 2023 •

edited

Loading