-
Notifications
You must be signed in to change notification settings - Fork 33
docs(navigation-primary): use rh-icon profile instead of rh-avatar #2395
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
base: main
Are you sure you want to change the base?
docs(navigation-primary): use rh-icon profile instead of rh-avatar #2395
Conversation
|
✅ Deploy Preview for red-hat-design-system ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Size Change: 0 B Total Size: 230 kB ℹ️ View Unchanged
|
|
Hm, I missed the fact that the icon stays the same in the authenticated state, and I don't think that's what the design intended. The icon that was in the nav mockup was different from the default icon for rh-avatar, so I had flagged that as something to change during RHDC's QA. I thought they were going to use that person icon in the unauthed state and rh-avatar in the authed state. On the rh-avatar side, is it possible to slot in a different default icon when the user is signed out? That might allow |
Could we update which icon represents |
this is preferable to having two different instances of what "avatar" means, one for nav-1, and a different one for everything else. There could even perhaps be a slot for the default icon in avatar |
|
this should maybe target diglett branch? |
This might actually be closed if we plan to make changes to |
|
@zeroedin @bennypowers, is it possible to change the |
|
Maybe with a slot |
|
@marionnegp @bennypowers are we suggesting the path forward with this is a slot addition to red-hat-design-system/elements/rh-avatar/rh-avatar.ts Lines 84 to 92 in adabbe6
Whereas if <canvas part="canvas"></canvas>` : this.src ? html`
<img src="${this.src}" role="presentation" part="img">` : html`
<slot name="icon">
<svg xmlns="http://www.w3.org/2000/svg" style="enable-background:new 0 0 36 36" viewBox="0 0 36 36" role="presentation" part="img" id="default">
<path d="M0 0h36v36H0z" class="st1"/><path d="M17.7 20.1c-3.5 0-6.4-2.9-6.4-6.4s2.9-6.4 6.4-6.4 6.4 2.9 6.4 6.4-2.8 6.4-6.4 6.4z" class="st3"/>
<path d="M13.3 36v-6.7c-2 .4-2.9 1.4-3.1 3.5l-.1 3.2h3.2z" class="st2"/>
<path d="m10.1 36 .1-3.2c.2-2.1 1.1-3.1 3.1-3.5V36h9.4v-6.7c2 .4 2.9 1.4 3.1 3.5l.1 3.2h4.7c-.4-3.9-1.3-9-2.9-11-1.1-1.4-2.3-2.2-3.5-2.6s-1.8-.6-6.3-.6-6.1.7-6.1.7c-1.2.4-2.4 1.2-3.4 2.6-1.7 1.9-2.6 7.1-3 10.9h4.7z" class="st4"/>
<path d="m25.9 36-.1-3.2c-.2-2.1-1.1-3.1-3.1-3.5V36h3.2z" class="st2"/>
</svg>
</slot>
`} |
|
The real problem here is that we have two different designs for avatars in the ds now: one for this specific case, and one for everything else. Best: just use Avatar, without changing the design, OR decide that avatar legitimately has this new variant (where else would it be used?) |
What I did
<rh-icon icon="profile" set="ui">is used instead of<rh-avatar>Testing Instructions
Notes to Reviewers
This is more a question of when we use
<rh-avatar>moving forward.It looks like the authenticated state of
<rh-avatar>may be restricted to internal use in the<rhx-account-dropdown>instead of a first-class citizen on the nav bar as it was prior primary nav designs. Instead, we are changing the text: Login to Account, and the icon stays the same. For compact viewport sizes, the profile icon remains with no other indication of the login status. @marionnegp @coreyvickery Is this correct?