Skip to content
This repository was archived by the owner on Jul 14, 2026. It is now read-only.

Include avatar placeholder in relevant places - #2783

Merged
PVince81 merged 2 commits into
masterfrom
fix-avatar-top-bar
Jan 7, 2020
Merged

Include avatar placeholder in relevant places#2783
PVince81 merged 2 commits into
masterfrom
fix-avatar-top-bar

Conversation

@PVince81

@PVince81 PVince81 commented Dec 20, 2019

Copy link
Copy Markdown
Contributor

Description

Pass in the user display name for the fallback logic that displays
letter.
Quick fix for alignment of avatar in top bar.
Update ODS for the fix for setting avatar size.
Use avatar placeholder component also in collaborator autocomplete and list.

Related Issue

None raised

Motivation and Context

Top bar looking shitty without proper alignment.
Avatar placeholder had missing letters.

How Has This Been Tested?

  • TEST: with placeholder: letters are shown, alignment is ok
  • TEST: with avatar picture: picture is shown in circle, alignment is ok

Screenshots (if appropriate):

image

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

@PVince81 PVince81 added Type:Bug Something isn't working Status:Needs-Review Needs review from a maintainer labels Dec 20, 2019
@PVince81
PVince81 requested a review from kulmann December 20, 2019 08:58
@PVince81 PVince81 self-assigned this Dec 20, 2019
@PVince81

Copy link
Copy Markdown
Contributor Author
  • investigate why button height is reduced in responsive mode

@PVince81 PVince81 changed the title Fix top bar avatar alignment Include avatar placeholder in relevant places Dec 20, 2019
@PVince81

Copy link
Copy Markdown
Contributor Author

fixed test errors because the avatar placeholder had included itself in the places where the test code is trying to read the user name

<oc-spinner v-if="loading" uk-spinner="ratio:1.6" class="uk-margin-small-right" :aria-label="$gettext('Loading avatar')" />
<template v-else>
<oc-avatar v-if="avatar" :src="avatar" class="uk-margin-small-right" width=50 height=50 />
<oc-avatar v-if="item.value.shareType === 0" :src="avatar" class="uk-margin-small-right" width=50 height=50 :userName="item.label" />

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

still has the obsolete height param

<oc-spinner v-if="loading" key="collaborator-avatar-spinner" uk-spinner="ratio:1.6" class="uk-margin-small-right" :aria-label="$gettext('Loading')"/>
<div v-else key="collaborator-avatar-loaded">
<oc-avatar v-if="avatar" :src="avatar" class="uk-margin-small-right" width=50 height=50 />
<oc-avatar v-if="collaborator.info.share_type === '0'" :src="avatar" class="uk-margin-small-right" size=50 :userName="collaborator.displayName" />

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should still be width instead of size, right?!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

indeed :-/

Vincent Petry added 2 commits January 7, 2020 10:40
Pass in the user display name for the fallback logic that displays
letter.
Quick fix for alignment of avatar in top bar.
Update ODS for the fix for setting avatar size.
@PVince81
PVince81 force-pushed the fix-avatar-top-bar branch from 6b72c4b to a30c6f8 Compare January 7, 2020 09:40
@PVince81

PVince81 commented Jan 7, 2020

Copy link
Copy Markdown
Contributor Author

@kulmann fixed.

Also I've fixed CI because now the avatar placeholder text was sneaking into the string that is read for assertions.

@LukasHirt LukasHirt left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Status:Needs-Review Needs review from a maintainer Type:Bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants