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

fix(NcAvatar) increase avatar sizes in call #10610

Merged
merged 5 commits into from
Oct 2, 2023

Conversation

Antreesy
Copy link
Contributor

☑️ Resolves

  • Fix follow-up: Reduce amount of component to wrap avatars #10282
    • native NcAvatar is no longer used in components
    • SCSS mixin avatar-mixin is no longer used in components
    • Size for all NcAvatar wrappers is now set explicitly - no more reactivity problems
  • Size of avatars is increased:
    • in MediaSettings: 128 => 180
    • in GridView and promoted VideoVue: 128 => 512
  • Common lint fixes
    • some SCSS variables were replaced with CSS variables provided by server

🖼️ Screenshots

🏚️ Before 🏡 After
image image
image image
image image

🚧 Tasks

  • Visual check
  • Code review

🏁 Checklist

Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
Comment on lines +209 to +210
color: #ffffff;
background-color: #b9b9b9;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest removing these static colors.
This is the view from Guest ( display name : pop )
image
And this is from a logged in user and Guest is present in the call ( display name : pop)
image

It would be nice if we use the same style for guest avatar from the user account view in the guest view ( make it always uppercase too)

Copy link
Contributor Author

@Antreesy Antreesy Oct 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These colors are also used in other places, to identify guest person in participants list or messages, for example:
image

Although I can't reproduce your second screenshot, Maybe, it's related to issue in second comment fixed

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This identification method might be okay in other places like messages and participant lists because it doesn't own much of "visual weight" but for call view, it occupies more visual surface.
Alternative : Add "( guest )" to the display name to identify the guest.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please reference all this in follow-up issue =)

src/components/CallView/shared/VideoVue.vue Outdated Show resolved Hide resolved
Copy link
Contributor

@DorraJaouad DorraJaouad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested 🦅
Avatar/space ratio looks better.

Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
@Antreesy Antreesy force-pushed the fix/10282/increase-avatars-in-call branch from 4f59ec8 to e4692fe Compare October 2, 2023 15:16
@Antreesy Antreesy merged commit cc903fc into master Oct 2, 2023
25 checks passed
@Antreesy Antreesy deleted the fix/10282/increase-avatars-in-call branch October 2, 2023 15:20
@Antreesy
Copy link
Contributor Author

Antreesy commented Oct 2, 2023

/backport to stable27

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.

follow-up: Reduce amount of component to wrap avatars
2 participants