fix: (core) toolbar enhancements

[artolshansky]

Please provide a link to the associated issue.

Closes: #2890
Closes: #3018

Please provide a brief summary of this pull request.

Please check whether the PR fulfills the following requirements

• the commit message follows the guideline:
  https://github.com/SAP/fundamental-ngx/blob/master/CONTRIBUTING.md
• tests for the changes that have been done
• all items on the PR Review Checklist are addressed :
  https://github.com/SAP/fundamental-ngx/wiki/PR-Review-Checklist

Documentation checklist:

• Documentation Examples
• Stackblitz works for all examples

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ olshansky
❌ salarenko
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[netlify]

Deploy preview for fundamental-ngx ready!

Built with commit 023808f

https://deploy-preview-3019--fundamental-ngx.netlify.app

[InnaAtanasova]

Looks good to me. I will not approve to give a chance to other people from the team to take a look. @JKMarkowski if you think it's ok you can approve and merge it.

[JKMarkowski]

Hi @olshansky Code looks great for me. Just one thing that bothers me

As you can see high/low priority are treated same. I guess this is also the reason why overflow button goes outside on lower resolutions

[artolshansky]

@JKMarkowski thanks, fixed

[sKudum]

Looks good. Just 2 points

• If we have accessibility also addressed with example it will help customers.
• We need to rebase it with the master.

[stefanoScalzo]

i dont think its normal that they are all disappearing with this much space left

[mikerodonnell89]

In the overflow priority example, "high" gets moved to overflow before "button first". I think any item without a specified priority should be moved before high priority items

[artolshansky]

@stefanoScalzo I believe that it's correct according to the specifications:

Child elements can be grouped so that they can "overflow" together.

That's why they have much space left

[artolshansky]

@mikerodonnell89 note from fiori design guidelines:

The priority of each item is high by default. If two items have equal priority, the item on the right side overflows first.

button first has the same priority as the button high, but the button high on the right side of button first. So, it's correct behavior

[JKMarkowski]

Priorities works well there. Great Job!

[artolshansky]

As for a11y

All child elements should be keyboard accessible

• User should be able to "tab" navigate through all child elements.
• User should be able to click "enter" to invoke action on the child elements.

The overflow menu should be keyboard accessible.

• User should be able to click "enter" to open the overflow menu.
• User should be able to navigate the overflow menu using "up" and "down" arrow keys.

---

• User should be able to click "enter" to invoke action on the child elements.

I think it's related to the implementation of each child component, because of components could have different behavior on occurred action (click, space or enter keypress)

• User should be able to navigate the overflow menu using "up" and "down" arrow keys.

It's a popover component (the same as in the previous paragraph) and could be implemented in a separate PR.

---

I could make a separate issue and describe these notes if necessary. Please, ping me if it necessary to do

[KevinOkamoto]

A minor issue - is there a way to get rid of the button flicker as the toolbar is resized? Can we add a check to see if the set of visible buttons change in the _collapseItems() function, or something similar?

[artolshansky]

@KevinOkamoto with checking it's not so easy as I expected. But I got rid of flickering similar as much as possible now to UI5 examples

[KevinOkamoto]

Looks good.