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

Add Persistence notification flags #786

Closed
wants to merge 3 commits into from

Conversation

3v1n0
Copy link
Contributor

@3v1n0 3v1n0 commented May 4, 2022

FDO Notifications had persistence flags, we don't support them in the portal, but applications may want to be able to still use them.

While transient notifications can be shown as they are, resident ones could instead be bothering an user, so add a further permission check in such case.

FDO Notifications could be set as transient and so only be shown without
being persistent if the desktop environment supported that.

It's still possible to support such parameter, as applications may want
to bother less the users.
@3v1n0 3v1n0 force-pushed the persistence-notification-flags branch 2 times, most recently from 3ce5078 to baa8b03 Compare May 5, 2022 15:50
3v1n0 added 2 commits May 5, 2022 17:51
These were supported by FDO Notifications protocol, but we don't support
them in the portal.

However, given that those can bother the user, adding a further
permission check, so that users can ignore such notifications flag.
@3v1n0 3v1n0 force-pushed the persistence-notification-flags branch from baa8b03 to d907d38 Compare May 5, 2022 15:51
@3v1n0
Copy link
Contributor Author

3v1n0 commented May 6, 2022

I wanted to add permission tests, but sadly this is a bit complex at the moment because of the xdp_app_info_is_host checks (which basically avoid any permission check).

So, I thought about defining a XDP_APP_INFO_KIND_TEST app that somewhat works, but requires further tests changes.

So, any suggestion here?

@3v1n0
Copy link
Contributor Author

3v1n0 commented Oct 30, 2022

@GeorgesStavracas hey, could you have a look at this?

And #791 maybe :-)

@3v1n0
Copy link
Contributor Author

3v1n0 commented Nov 3, 2022

So, I thought about defining a XDP_APP_INFO_KIND_TEST app that somewhat works, but requires further tests changes.

So, the paradigm to make things more testable is in #904, so once that lands this will be easily testable.

@GeorgesStavracas GeorgesStavracas added new api This requires adding API to an existing portal portal: notifications Notifications portal labels Oct 3, 2023
@swick
Copy link
Contributor

swick commented Oct 9, 2024

@3v1n0 is this still relevant with the Notifications v2 portal landing soon?

@jsparber
Copy link
Contributor

I think this can be closed since notification v2 has a new property display-hint which does the same thing as proposed here. See: #1298

@GeorgesStavracas
Copy link
Member

Thanks @jsparber for checking this. @3v1n0 as always feel free to reopen if you think this still applies!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new api This requires adding API to an existing portal portal: notifications Notifications portal
Projects
No open projects
Status: Triaged
Development

Successfully merging this pull request may close these issues.

4 participants