Skip to content

Add option to enable notifications#8458

Merged
Alkarex merged 11 commits intoFreshRSS:edgefrom
rupakbajgain:notification-button
Jan 29, 2026
Merged

Add option to enable notifications#8458
Alkarex merged 11 commits intoFreshRSS:edgefrom
rupakbajgain:notification-button

Conversation

@rupakbajgain
Copy link
Contributor

@rupakbajgain rupakbajgain commented Jan 22, 2026

Closes #7330

Changes proposed in this pull request:

  • Default behavior is same
  • Added FreshRSS_Context::userConf()->html5_disable_notif so that, it determines weather user wants notification. (will not show any even it has permission) (i want default false so disable, so old configs get proper default values)
  • Added button such that checking it makes it request permission too
image
  • clear commit messages

  • code manually tested

  • test notification actually happening (how can i trigger it, do i have to wait it), this code fixes permissions.

Inverted variable name so permission remains fixed

Proper name
FreshRSS_Context::userConf()->show_nav_buttons = Minz_Request::paramBoolean('show_nav_buttons');
FreshRSS_Context::userConf()->html5_notif_timeout = max(0, Minz_Request::paramInt('html5_notif_timeout'));

FreshRSS_Context::userConf()->html5_disable_notif = !Minz_Request::paramBoolean('html5_enable_notif');
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer a positive parameter such as html5_notif_enabled if possible

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Alkarex i started with default positive.
But there was small issue,
Old user config would have their notification turned off, because this variable is false.
I can revert it to old code.

#8457

Copy link
Member

Choose a reason for hiding this comment

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

Even if set to true in config-user.default.php?

@Alkarex Alkarex added this to the 1.29.0 milestone Jan 22, 2026
@Alkarex Alkarex added UI 🎨 User Interfaces UX User experience labels Jan 22, 2026
@Alkarex
Copy link
Member

Alkarex commented Jan 28, 2026

I have made several fixes. Please test again

@rupakbajgain
Copy link
Contributor Author

ok. manually tested it. same behavior.

@Alkarex Alkarex merged commit b59a210 into FreshRSS:edge Jan 29, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

UI 🎨 User Interfaces UX User experience

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PWA request for notifications

2 participants