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

ui5-notification-li: new component #1478

Closed
codefactor opened this issue Apr 14, 2020 · 10 comments · Fixed by #1576
Closed

ui5-notification-li: new component #1478

codefactor opened this issue Apr 14, 2020 · 10 comments · Fixed by #1576

Comments

@codefactor
Copy link
Contributor

codefactor commented Apr 14, 2020

Is your feature request related to a problem? Please describe.
The SuccessFactors UI5 header supports a Notification List for displaying notification items when the "bell" icon is clicked. The Notification List which is part of sap.m library has no UI5 web component equivalent.

Here is a link to sample in sap.m:
http://veui5infra.dhcp.wdf.sap.corp:8080/sapui5-sdk-dist/#/entity/sap.m.NotificationListItem/sample/sap.m.sample.NotificationListItem

Describe the solution you'd like
At least the usage of NotificationList in SuccessFactors should be supported - but ideally the entire functionality of NotificationListItem/Group should be supported.

Describe alternatives you've considered
Using the custom list item and styling the contents manually.

Additional context
Add any other context or screenshots about the feature request here.

This is our UI5 version of the notification list (showing with sample data):
image

@MarcusNotheis
Copy link
Collaborator

Hi @codefactor,
in case you are using React in your application you might also look into the Notification and NotificationGroup Components from the Web Component for React wrapper.
This might help you to bridge the time until the component is available as Web Component.

@codefactor
Copy link
Contributor Author

@MarcusNotheis @ilhan007 @vladitasev ,

We are thinking to add this to a SuccessFactors web component version of the header which needs to be runnable from arbitrary page - it could be UI5 or React or legacy no-framework page. So using React in this case would be inconvenient. Though some of our pages do use React.

I wonder, is there any chance this might be started soon?

Also, I wonder, if this is a good or bad example of something that SuccessFactors team could consider to contribute to ui5-webcomponents as open source contributor? We are taking a look at this among other possibilities.

@vladitasev
Copy link
Contributor

This will be prioritized for the next public release (RC8), but hopefully finished much sooner (before mid of May)

@vladitasev
Copy link
Contributor

vladitasev commented Apr 28, 2020

Note: the design has changed:

image

We'll implement the latest Fiori design, as currently shown in the screenshot.

New component: ui5-notification-li in the fiori package:

  • slot for avatar
  • properties: heading, icon
  • default slot is the content - completely free
  • properties for close button and overflow button. TODO: determine the most convenient interface for providing overflow menu options. Could be similar to shellbar items for the different menus.
  • properties for the footer items, but not semantic such as "duration", etc... we just need to provide interface for the dots between them and styling.
  • support for "Show more/show less"

The new component is a list item.

As discussed, notification list group will be part of the initial scope.

Edit: scope adjusted to reflect that there will be no selection modes and other list-related functionality, that is not applicable to notification list items.

@vladitasev vladitasev changed the title Notification List - Feature ui5-notification-li: new component Apr 28, 2020
@vladitasev
Copy link
Contributor

Update: scope has changed.

Notification list group will be part of the initial scope.

ilhan007 added a commit that referenced this issue May 13, 2020
…omponents (#1576)

The component covers the main functionaly of the sap.m.NotificationListItem.
It is meant to display a notification and has a rich set of properties and slots to do it.

Fixes: #1478
@codefactor
Copy link
Contributor Author

codefactor commented May 16, 2020

@ilhan007 ,

It looks pretty good.
image

If you compare this with the original, there are only a few little differences:

  1. The group header has less room because the button "See 1 old" is not on the next line
  2. I couldn't find a place to put the text "Last: 4 months" in the group header -- which I think is the sap.m.NotificationListGroup#datetime property. In my above example, though, it would say "Last: 4 days" though.

@codefactor
Copy link
Contributor Author

codefactor commented May 16, 2020

@ilhan007 ,

One more thing - I wasn't able get it to work with a "busy-indicator" on any level outside of the parent level. To explain, when the user does something, such as mark an item as read or click on the "See 1 old" button, these activate asynchronous actions that may take some time. In OpenUI5 we could bind against the busy property, but in UI5 web components we can only wrap the thing we want with a <ui5-busyindicator> which doesn't work properly in this case because of CSS issues. I also had a similar issue at the whole <ui5-list> level but in that case I was able to fix the CSS issues by adding some width:100% at certain levels.

You can have a try by trying the following github:

git clone https://github.wdf.sap.corp/xweb/common-components
git co feature/header
cd common-components/packages/webcomponents
npm i
npm start

Then open http://localhost:8081/test-resources/pages/index.html?delay=3000

This will add artificial 3 second delay to the mocks and then open the notification side panel, and try out the "See 1 Old" button.

In the file:
https://github.wdf.sap.corp/xweb/common-components/blob/feature/header/packages/webcomponents/src/NotificationList.lit.js

You will find 2 places where the busy flag is passed down:
renderNotificationGroup
renderNotificationItem

In these cases they are given the busy, or parentBusy - where parentBusy is the busy flag on the group level and busy is the flag on the item level. So in my case those busy flags are not utilized right now -- you could try to add the <ui5-busyindicator> wrappers around the element and set the enabled flag on them based on these flags; however, you will find that the layout of the notification changes.

Can you please have a try and let me know if you can easily solve this issue with a quick fix?
Or maybe it would be a good idea to add a new "busy" flag and then use the <ui5-busyindicator> inside the shadow DOM instead?

@ilhan007
Copy link
Member

Hello @codefactor

  1. sap.m.NotificationListGroup "datetime" property

It seems the NotificationListGroup "datetime" property is deprecated.

https://openui5.hana.ondemand.com/api/sap.m.NotificationListGroup#controlProperties
Screenshot 2020-05-17 at 9 57 51 AM

Also, I tried the nightly version, set "datetime", but it does not show up anywhere: http://veui5infra.dhcp.wdf.sap.corp:8080/snippix/#46822

  1. sap.m.NotificationListGroup actions

I checked sap.m.NotificationListGroup and it behaves the same, neither the heading of the group item wraps, nor the actions. See the "Accept all" button does not wrap too.

Screenshot 2020-05-17 at 10 10 33 AM

Best,
ilhan

@ilhan007
Copy link
Member

Hello @codefactor
regarding setting busy state, you want to be able to make busy particular group and/or particular item right?

@codefactor
Copy link
Contributor Author

@ilhan007 ,
Yes, that’s right.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants