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

Display user status by the side in sharing flow #40393

Merged
merged 2 commits into from
Sep 13, 2023

Conversation

nfebe
Copy link
Contributor

@nfebe nfebe commented Sep 13, 2023

The quick share drop down now takes the place where the user status would show in the past, so we have the option of removing it (as it's available in the profile dropdown) or putting it at the side as such.

It's put on the side without the icon, as the status icon appears on the avatar when set.

Before Now
Screenshot from 2023-09-13 11-32-46 Screenshot from 2023-09-13 12-00-20

@nfebe
Copy link
Contributor Author

nfebe commented Sep 13, 2023

Updates that take into consideration disambiguation text as well as wrap. (Also blurred the text, just like the disambiguation text)

Without wrap With wrap
Screenshot from 2023-09-13 15-07-07 Screenshot from 2023-09-13 15-07-48

The quick share drop down now takes the place where the user status
would show in the past, so we have the option of removing it (as it's available
in the profile dropdown) or putting it at the side as such.

It's put on the side without the icon, as the status icon appears on the avatar
when set.

Signed-off-by: fenn-cs <[email protected]>
@nfebe nfebe force-pushed the fix-user-status-sharing-flow branch from 32c4c01 to c0344a2 Compare September 13, 2023 14:19
@nfebe nfebe force-pushed the fix-user-status-sharing-flow branch from c0344a2 to 7d9b5b6 Compare September 13, 2023 14:34
Removes button from footer and removes button background.

Signed-off-by: fenn-cs <[email protected]>
@AndyScherzinger
Copy link
Member

Looks good, just some minor issues as mentioned partially already

  • is Ellipsizing implemented?
  • text size of the parenthesis text is not yet unified, see status vs. mail address for example
  • is the grey tone accessible? Given the font size the contrast ratio needs to be 1:4,5 (relevant more for Master than stable27)

@nfebe nfebe force-pushed the fix-user-status-sharing-flow branch from 7d9b5b6 to 2b2622d Compare September 13, 2023 14:43
@nfebe
Copy link
Contributor Author

nfebe commented Sep 13, 2023

is Ellipsizing implemented?

So no wrap? things wrap in a decent way when text is too long as shown in the last screen shots.

text size of the parenthesis text is not yet unified, see status vs. mail address for example
is the grey tone accessible? Given the font size the contrast ratio needs to be 1:4,5 (relevant more for Master than stable27)

color: var(--color-text-maxcontrast); is used.

cc: @AndyScherzinger

@jancborchardt
Copy link
Member

Sorry but with wrapping it looks way too wonky, we shouldn't do that. It needs to be ellipsized to one line. :)

Also, the gap between the first line and the permission line is too big for users. For "Share link" it looks good, but for the users below it's larger.

@AndyScherzinger
Copy link
Member

@fenn-cs Color value in use looks good 👍
Ellipsizing like mentioned by Jan, yeah, preferred to enforce single line for that part of the list item's data.

@nfebe nfebe mentioned this pull request Sep 13, 2023
@blizzz blizzz merged commit 80f3e46 into master Sep 13, 2023
40 checks passed
@blizzz blizzz deleted the fix-user-status-sharing-flow branch September 13, 2023 19:36
@nfebe
Copy link
Contributor Author

nfebe commented Sep 13, 2023

/backport to stable27

@backportbot-nextcloud
Copy link

The backport to stable27 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable27
git pull origin stable27

# Create the new backport branch
git checkout -b fix/foo-stable27

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts. Resolve them.
git cherry-pick abc123

# Push the cherry pick commit to the remote repository and open a pull request
git push origin fix/foo-stable27

More info at https://docs.nextcloud.com/server/latest/developer_manual/getting_started/development_process.html#manual-backport

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants