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

feat(core): shellbar redesign, add branding and additional context area #12694

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

dpavlenishvili
Copy link
Contributor

@dpavlenishvili dpavlenishvili commented Nov 6, 2024

Related Issue(s)

closes #12520

Description

Implemented Shellbar ContextArea and integrated Shellbar Branding.

  • Added customizable branding section with logo, title, and subtitle via fd-shellbar-branding
  • Introduced fd-shellbar-context-area for additional elements like separator and spacer
  • Implemented fdShellbarHidePriority directive for custom overflow management with unique priority values
  • Enabled automatic priority assignment for context area elements based on placement order when fdShellbarHidePriority is omitted
  • Action Overflow Toolbar: Integrated an overflow toolbar for Shellbar actions, ensuring responsive behavior when space is limited. Actions are moved into the overflow toolbar dynamically based on available space.

Screenshots

Before:

After:

image

@dpavlenishvili dpavlenishvili requested a review from a team November 6, 2024 12:03
@dpavlenishvili dpavlenishvili self-assigned this Nov 6, 2024
@dpavlenishvili dpavlenishvili linked an issue Nov 6, 2024 that may be closed by this pull request
2 tasks
Copy link

netlify bot commented Nov 6, 2024

Deploy Preview for fundamental-ngx ready!

Name Link
🔨 Latest commit 0a31d70
🔍 Latest deploy log https://app.netlify.com/sites/fundamental-ngx/deploys/676984cd84fbad0008b6114a
😎 Deploy Preview https://deploy-preview-12694--fundamental-ngx.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

github-actions bot commented Nov 6, 2024

Visit the preview URL for this PR (updated for commit 0a31d70):

https://fundamental-ngx-gh--pr12694-12520-shellbar-o0tkbkh1.web.app

(expires Thu, 26 Dec 2024 15:46:53 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 41b993ee8e451bd7c6770b342ce142dc886eacff

@khotcholava
Copy link
Contributor

@dpavlenishvili
Looks good overall! Just one thing:

When I open the menu on a small screen, such as "Corporate Portal" (as shown in the video), the first menu item is selected by default. However, when I open the notification menu (three dots), the first item is not automatically selected. Is this intentional, or should the first item be selected there as well?

Screen.Recording.2024-11-25.at.17.00.12.mov

@droshev droshev requested a review from a team November 29, 2024 14:32
@InnaAtanasova
Copy link
Contributor

Screenshot 2024-11-29 at 2 13 03 PM

Empty elements shouldn't be rendered.

@InnaAtanasova
Copy link
Contributor

Screenshot 2024-11-29 at 2 16 24 PM

The Branding component needs a complete re-write. The markup is very wrong. Apart from having too many elements, it will cause big a11y issues. There are repeated classes that cause styling issues.

@InnaAtanasova
Copy link
Contributor

Screenshot 2024-11-29 at 2 22 01 PM

The Search component has a couple of issues:

  1. Per design specs, it should always be positioned on the right
Screenshot 2024-11-29 at 2 38 12 PM 2. You can't have the search icon and the expanded version of the search. The expanded version is shown when you click on the icon. On screen resize, when the space becomes limited, the expanded version goes back to an icon.

@InnaAtanasova
Copy link
Contributor

InnaAtanasova commented Nov 29, 2024

Cont'd of the Search issue:

Screen.Recording.2024-11-29.at.2.26.42.PM.mov

When you hide the search, there's no Search icon that appears. You just completely hide the element. The icon should be visible

@InnaAtanasova
Copy link
Contributor

Screen.Recording.2024-11-29.at.2.29.48.PM.mov

Here as well (Toolbar implementation), the Search is on the wrong position, it hides completely when there isn't enough space and the search icon is missing.

On resize the paddings on the left and right of the whole container should change. You need to apply the modifier classes on the Shellbar component. See Responsive paddings in Fundamental-styles:
Screenshot 2024-11-29 at 2 45 12 PM

@InnaAtanasova
Copy link
Contributor

The overflow menu should look like this (design):
Screenshot 2024-11-29 at 2 27 35 PM

Now it looks like this:
Screenshot 2024-11-29 at 2 27 42 PM

@InnaAtanasova
Copy link
Contributor

Screenshot 2024-11-29 at 2 49 03 PM

There should be an example with a hamburger menu in front of the logo.

@InnaAtanasova
Copy link
Contributor

Screenshot 2024-11-29 at 2 51 33 PM

On M screen, when the Search is expanded, the content overflows

@InnaAtanasova
Copy link
Contributor

Screenshot 2024-11-29 at 2 54 02 PM

The elements shouldn't go to an overflow menu if there's enough space.

@droshev
Copy link
Contributor

droshev commented Nov 30, 2024

@dpavlenishvili do we have breaking changes? If so, they need to be documented as well.

@droshev
Copy link
Contributor

droshev commented Dec 18, 2024

@dpavlenishvili check the current status, investigate whether you can do the overflow menu and write in the PR the status.

@droshev
Copy link
Contributor

droshev commented Dec 19, 2024

@dpavlenishvili check the current status, investigate whether you can do the overflow menu and write in the PR the status.

@dpavlenishvili
Copy link
Contributor Author

dpavlenishvili commented Dec 20, 2024

already done:
new structure for branding, title, subtitle and new way to pass branding logo, context area.

what need to be done:
overflow menu, i think that only one way is that to create new overflow menu for shellBar.

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.

UXC: shellbar updates
4 participants