feat: Avatar Group overflow spacing and overflow button border color #2121

artolshansky · 2021-02-05T12:27:33Z

Related Issue

Closes #2091
Part of #2153

Description

Added overflow spacing and overflow button accent border.

Please check whether the PR fulfills the following requirements

The output matches the design specs

The code follows fundamental-styles code standards and style

only one top level fd-* class is used in the file
BEM naming convention is used
Mixins are used for repeatable code (fd-rtl, fd-ellipsis, fd-flex, fd-selected, fd-focus, ect.)
A11y support - keyboard support, screenreader support, proper ARIA attributes, etc.
fd-reset() mixin is applied to all elements
Variables are used, if some value is used more than twice.
Checked if current components can be reused, instead of having new code.

Testing

last commit message should have [ci visual] so it can trigger chromatic visual regression (e.g. test: run chromatic visual regression [ci visual])
[n/a] tested Storybook examples with "CSS Resources" normalize option
[n/a] tested Storybook examples with "CSS Resources" unnormalize option
Verified all styles in IE11
Updated tests

Documentation

Storybook documentation has been created/updated
[n/a] Breaking Changes wiki has been updated in case of breaking changes.

netlify · 2021-02-05T12:31:29Z

Deploy preview for fundamental-styles ready!

Built with commit 6571f73

https://deploy-preview-2121--fundamental-styles.netlify.app

netlify · 2021-02-05T12:36:16Z

Deploy preview for fundamental-styles ready!

Built with commit d1cf59f

https://deploy-preview-2121--fundamental-styles.netlify.app

mikerodonnell89 · 2021-02-05T19:02:32Z

src/avatar-group.scss

+  &__overflow {
+    @include fd-reset();
+
+    &__body {


results in fd-avatar-group__overflow__body which is not valid BEM, fd-avatar-group__overflow-body is fine

@mikerodonnell89 fixed

InnaAtanasova · 2021-02-05T19:26:32Z

src/avatar-group.scss

@@ -81,6 +81,19 @@ $block: #{$fd-namespace}-avatar-group;
    }
  }

+  &__overflow {


why do you need this? It's used like this <div class="fd-popover fd-avatar-group__overflow"> and it already has a reset coming from fd-popover. If you inspect the element, all of the rules are overwritten:

You can only use fd-avatar-group__overflow-body where you apply the 1rem padding and use the specificity to add margin to the item.

@InnaAtanasova fixed

InnaAtanasova · 2021-02-10T17:22:20Z

src/avatar-group.scss

@@ -81,6 +81,17 @@ $block: #{$fd-namespace}-avatar-group;
    }
  }

+  &__overflow--body {


should be &__overflow-body

__overflow--body means that there's a class __overflow and --body is modifying this class

@InnaAtanasova fixed

mikerodonnell89

High contrast colors for the "+8" in group mode are incorrect

Also, the margin-right in individual mode is correctly set to 0.125rem, but there is some additional space before the next avatar so I think they may still be spread too far apart

artolshansky · 2021-02-11T10:08:09Z

High contrast colors for the "+8" in group mode are incorrect

@mikerodonnell89 why it's incorrect? The button has a color-accent-6 and as I see avatar has the same colors in HC mode

Also, the margin-right in individual mode is correctly set to 0.125rem, but there is some additional space before the next avatar so I think they may still be spread too far apart

fixed

mikerodonnell89 · 2021-02-17T01:38:28Z

@mikerodonnell89 why it's incorrect? The button has a color-accent-6 and as I see avatar has the same colors in HC mode

@olshansky screenshot from the specs:

So I think we will need a new style for this overflow avatar with the + text

artolshansky · 2021-02-17T09:35:43Z

@olshansky screenshot from the specs:

So I think we will need a new style for this overflow avatar with the + text

@mikerodonnell89 screenshot from the examples (I just removed accent color class modifier). Looks correct, isn't it?
FYI this button has a color different color accent in examples just to show that we could use color accents modifier classes for this button.

InnaAtanasova · 2021-02-18T20:23:12Z

@olshansky are these changes intended?

artolshansky · 2021-02-19T07:41:09Z

@olshansky are these changes intended?

@InnaAtanasova old service for placeholder images don't work (that's why I changed it at the last commit)

droshev · 2021-02-19T12:33:00Z

@olshansky can you address #2153 as part of this ticket?

artolshansky · 2021-02-19T14:40:26Z

@droshev should it be part of this PR? Because it is an issue in the popover component

droshev · 2021-02-20T00:27:04Z

@droshev should it be part of this PR? Because it is an issue in the popover component

what is the problem with the popover?

artolshansky · 2021-02-22T08:49:30Z

what is the problem with the popover?

@droshev Popover header border color is different than the body border in Dark theme

droshev · 2021-02-22T20:40:50Z

what is the problem with the popover?

@droshev Popover header border color is different than the body border in Dark theme

my comment was related to the arrow in RTL - it wasn't centered

https://user-images.githubusercontent.com/4380815/108005485-d86a8780-6fc6-11eb-9f0a-d0d91f6d454f.png

artolshansky · 2021-02-24T08:10:09Z

my comment was related to the arrow in RTL - it wasn't centered

https://user-images.githubusercontent.com/4380815/108005485-d86a8780-6fc6-11eb-9f0a-d0d91f6d454f.png

@droshev Oh yes, fixed

artolshansky added the Enhancement New feature or request label Feb 5, 2021

artolshansky added this to the Sprint 56 - ariba milestone Feb 5, 2021

artolshansky requested review from a team February 5, 2021 12:27

artolshansky self-assigned this Feb 5, 2021

artolshansky force-pushed the fix/avatar-group-overflow-spacing branch from 6571f73 to bd5574d Compare February 5, 2021 12:29

artolshansky force-pushed the fix/avatar-group-overflow-spacing branch from bd5574d to 8927c21 Compare February 5, 2021 12:46

mikerodonnell89 requested changes Feb 5, 2021

View reviewed changes

InnaAtanasova requested changes Feb 5, 2021

View reviewed changes

artolshansky requested review from mikerodonnell89 and InnaAtanasova February 8, 2021 07:26

InnaAtanasova reviewed Feb 10, 2021

View reviewed changes

mikerodonnell89 requested changes Feb 10, 2021

View reviewed changes

artolshansky requested review from InnaAtanasova and mikerodonnell89 February 11, 2021 10:08

InnaAtanasova approved these changes Feb 11, 2021

View reviewed changes

manjunathanagaraj modified the milestones: Sprint 56 - ariba, Sprint 56a - ariba Feb 15, 2021

artolshansky force-pushed the fix/avatar-group-overflow-spacing branch from 1f16e2e to 386f1a3 Compare February 17, 2021 15:15

mikerodonnell89 approved these changes Feb 17, 2021

View reviewed changes

artolshansky added 5 commits February 24, 2021 10:06

fix: Avatar Group overflow spacing [ci visual]

d07e42d

fix: drop redundant element [ci visual]

29bda4f

fix comments [ci visual]

b99ecc4

fix: remove accent color modifier class in example [ci visual]

11adc5e

fix: rtl positioning [ci visual]

d1cf59f

artolshansky force-pushed the fix/avatar-group-overflow-spacing branch from 4769452 to d1cf59f Compare February 24, 2021 08:09

artolshansky mentioned this pull request Feb 24, 2021

Visual Issues - Avatar Group #2153

Closed

2 tasks

artolshansky merged commit b20b90a into main Feb 26, 2021

artolshansky deleted the fix/avatar-group-overflow-spacing branch February 26, 2021 07:19

manjunathanagaraj mentioned this pull request Feb 26, 2021

Sprint 56a- summary SAP/fundamental-ngx#4780

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Avatar Group overflow spacing and overflow button border color #2121

feat: Avatar Group overflow spacing and overflow button border color #2121

artolshansky commented Feb 5, 2021 •

edited

netlify bot commented Feb 5, 2021

netlify bot commented Feb 5, 2021 •

edited

mikerodonnell89 Feb 5, 2021

artolshansky Feb 8, 2021

InnaAtanasova Feb 5, 2021

artolshansky Feb 8, 2021

InnaAtanasova Feb 10, 2021

InnaAtanasova Feb 10, 2021

artolshansky Feb 11, 2021

mikerodonnell89 left a comment

artolshansky commented Feb 11, 2021

mikerodonnell89 commented Feb 17, 2021

artolshansky commented Feb 17, 2021

InnaAtanasova commented Feb 18, 2021

artolshansky commented Feb 19, 2021

droshev commented Feb 19, 2021

artolshansky commented Feb 19, 2021

droshev commented Feb 20, 2021

artolshansky commented Feb 22, 2021 •

edited

droshev commented Feb 22, 2021 •

edited

artolshansky commented Feb 24, 2021 •

edited

feat: Avatar Group overflow spacing and overflow button border color #2121

feat: Avatar Group overflow spacing and overflow button border color #2121

Conversation

artolshansky commented Feb 5, 2021 • edited

Related Issue

Description

Please check whether the PR fulfills the following requirements

netlify bot commented Feb 5, 2021

netlify bot commented Feb 5, 2021 • edited

mikerodonnell89 Feb 5, 2021

Choose a reason for hiding this comment

artolshansky Feb 8, 2021

Choose a reason for hiding this comment

InnaAtanasova Feb 5, 2021

Choose a reason for hiding this comment

artolshansky Feb 8, 2021

Choose a reason for hiding this comment

InnaAtanasova Feb 10, 2021

Choose a reason for hiding this comment

InnaAtanasova Feb 10, 2021

Choose a reason for hiding this comment

artolshansky Feb 11, 2021

Choose a reason for hiding this comment

mikerodonnell89 left a comment

Choose a reason for hiding this comment

artolshansky commented Feb 11, 2021

mikerodonnell89 commented Feb 17, 2021

artolshansky commented Feb 17, 2021

InnaAtanasova commented Feb 18, 2021

artolshansky commented Feb 19, 2021

droshev commented Feb 19, 2021

artolshansky commented Feb 19, 2021

droshev commented Feb 20, 2021

artolshansky commented Feb 22, 2021 • edited

droshev commented Feb 22, 2021 • edited

artolshansky commented Feb 24, 2021 • edited

artolshansky commented Feb 5, 2021 •

edited

netlify bot commented Feb 5, 2021 •

edited

artolshansky commented Feb 22, 2021 •

edited

droshev commented Feb 22, 2021 •

edited

artolshansky commented Feb 24, 2021 •

edited