Skip to content

Conversation

@hehoon
Copy link
Collaborator

@hehoon hehoon commented Jan 5, 2026

I have JIRA issue created

  • branch and/or PR name(s) includes JIRA ID
  • issue has "Fix version" assigned
  • issue "Status" is set to "In review"
  • PR labels are selected
  • FLP integration tests were ran successful

Changes:

  • Add helpers to get, request, and check browser notification permissions

@hehoon hehoon force-pushed the feature/FRM/OGUI-1854/native-browser-notification-helpers branch from fb89f92 to d05ba52 Compare January 6, 2026 14:03
@hehoon hehoon marked this pull request as ready for review January 6, 2026 14:03
@hehoon hehoon requested a review from graduta as a code owner January 6, 2026 14:03
* Calling this method directly is discouraged; it may be removed in future versions.
* @returns {boolean} Returns `true` if context is secure
*/
isContextSecure() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this method is private and used within the CopyToClipboardComponent.js only, could you please rather than deprecate it, update it to use the one you recommend?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this is not a class, the naming should be browserNotification maybe even adding something to point out is a utility package.

Given we also have BrowserStorage, maybe you want to create a directory for browser only and keep all related in there

}

const permission = getBrowserNotificationPermission();
if (permission === 'granted') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that the permission value can only be: granted, default, denied, I think it would be good to have an object with these values in the file and reuse it from there.
In this way you can also use it as reference in docs with that getBrowserNotificationPermission can return

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

Development

Successfully merging this pull request may close these issues.

3 participants