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(ui5-avatar): avatar initials correct display #6731

Merged
merged 1 commit into from
Apr 28, 2023

Conversation

PetyaMarkovaBogdanova
Copy link
Contributor

@PetyaMarkovaBogdanova PetyaMarkovaBogdanova commented Mar 17, 2023

Fixes: #6585
Fixes: #6516

@PetyaMarkovaBogdanova PetyaMarkovaBogdanova marked this pull request as ready for review March 17, 2023 14:27
Copy link
Contributor

@nnaydenow nnaydenow left a comment

Choose a reason for hiding this comment

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

Change PR title to be ui5-avatar related. Testing it locally the change doesn't work as expected. If fallback icon is set initials container avatar.querySelector(".ui5-avatar-initials") return null and the expression is never done.

@PetyaMarkovaBogdanova
Copy link
Contributor Author

Change PR title to be ui5-avatar related. Testing it locally the change doesn't work as expected. If fallback icon is set initials container avatar.querySelector(".ui5-avatar-initials") return null and the expression is never done.

In what case did you see the fallback icon being set?

@nnaydenow
Copy link
Contributor

nnaydenow commented Mar 29, 2023

Change PR title to be ui5-avatar related. Testing it locally the change doesn't work as expected. If fallback icon is set initials container avatar.querySelector(".ui5-avatar-initials") return null and the expression is never done.

In what case did you see the fallback icon being set?

Using the initial from the original issue "RHB". If you change the initials to "RH" for example the fallback icon will not be removed as expected. Check initials from element inspector https://codesandbox.io/s/ui5-webcomponents-forked-6o4xi1?file=/index.html

@PetyaMarkovaBogdanova
Copy link
Contributor Author

@nnaydenow , can you, please, check my new solution?

@PetyaMarkovaBogdanova PetyaMarkovaBogdanova changed the title fix(ui5-shellbar): avatar initials display #6642 fix(ui5-avatar): avatar initials correct display #6642 Apr 3, 2023
@nnaydenow nnaydenow changed the title fix(ui5-avatar): avatar initials correct display #6642 fix(ui5-avatar): avatar initials correct display Apr 5, 2023
@nnaydenow
Copy link
Contributor

  1. In main initials are with lower priority than icon (if icon attribute exist => show icon). Currently if initials are set the icon will not appear.
  2. In main if invalid initials are used the fallback icon is shown. Currently if we set for example ЦИ empty avatar will appear.
  3. Resize handler is not called if the avatar is defined as and initials are set on a later stage:
<ui5-avatar id="test" size="XS" icon="share"></ui5-avatar>
<script>
	const test = document.getElementById("test");
	setTimeout(() => test.initials = "AC", 3000);
</script>

@PetyaMarkovaBogdanova PetyaMarkovaBogdanova force-pushed the fix-avatar-initials-display branch from 68dbdff to 78696d6 Compare April 7, 2023 06:53
Copy link
Contributor

@nnaydenow nnaydenow left a comment

Choose a reason for hiding this comment

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

<ui5-avatar initials="RHB" size="XS"></ui5-avatar>

resolves to:

<ui5-avatar initials="RHB" size="XS" icon="employee"></ui5-avatar>

With this setup icon can't be removed.

Screen.Recording.2023-04-07.at.10.25.47.mov
Screen.Recording.2023-04-07.at.10.39.55.mov

@PetyaMarkovaBogdanova PetyaMarkovaBogdanova force-pushed the fix-avatar-initials-display branch from 78696d6 to 091575f Compare April 7, 2023 09:08
@PetyaMarkovaBogdanova
Copy link
Contributor Author

@nnaydenow I experienced the same issue, but it seemed unlogical compared to the new code, so I rebuild and reloaded WC project. Can you, please, try doing that and test again, the issue is not happening on my side.

@nnaydenow
Copy link
Contributor

@nnaydenow I experienced the same issue, but it seemed unlogical compared to the new code, so I rebuild and reloaded WC project. Can you, please, try doing that and test again, the issue is not happening on my side.

Seems like I haven't rebased successfully. Works okey for me.

@PetyaMarkovaBogdanova PetyaMarkovaBogdanova force-pushed the fix-avatar-initials-display branch from 091575f to 9a8355c Compare April 20, 2023 12:32
@PetyaMarkovaBogdanova PetyaMarkovaBogdanova merged commit 16e6307 into main Apr 28, 2023
NHristov-sap pushed a commit that referenced this pull request May 4, 2023
fix(ui5-shellbar): avatar initials display #6642
PetyaMarkovaBogdanova added a commit that referenced this pull request Jun 20, 2023

Verified

This commit was signed with the committer’s verified signature.
arvidfm Arvid Fahlström Myrman
fix(ui5-shellbar): avatar initials display #6642
@ilhan007 ilhan007 deleted the fix-avatar-initials-display branch July 11, 2023 06:11
ilhan007 pushed a commit that referenced this pull request Jul 11, 2023

Verified

This commit was signed with the committer’s verified signature.
arvidfm Arvid Fahlström Myrman
fix(ui5-shellbar): avatar initials display #6642
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants