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

Feature/59793 Add new icons for setting reminders op-alarm & op-alarm-set #17497

Merged
merged 4 commits into from
Dec 19, 2024

Conversation

akabiru
Copy link
Member

@akabiru akabiru commented Dec 18, 2024

Ticket

https://community.openproject.org/work_packages/59793

What are you trying to accomplish?

Update reminder icon from clock to new op-alarm and op-alarm-set icons.

Screenshots

WorkPackage Overview

Kapture.2024-12-19.at.00.09.08.mp4

Notification Centre

Screenshot 2024-12-19 at 12 06 33 AM

What approach did you choose and why?

Replace reminderCount$:Observable<number> with hasReminder$:Observable<boolean>; further the hasReminder observable emission is standardised to depend on two actions: ReminderModalUpdated and notificationCountChanged

Merge checklist

  • Added/updated tests
  • Added/updated documentation in Lookbook (patterns, previews, etc)
  • Tested major browsers (Chrome, Firefox, Edge, ...)

Consequently attempt to simplify the observable structure:
 hasReminder$ should emit based if notificationCountChanged$ | reminderModalUpdated$ emit
 i.e. if there's a new notification (from polling) or when the reminder modal is closed we want
 to recheck the reminderCount and subsequently update the icon
@akabiru akabiru added this to the 15.2.x milestone Dec 18, 2024
@akabiru akabiru self-assigned this Dec 18, 2024
@akabiru akabiru marked this pull request as ready for review December 18, 2024 22:08
@akabiru akabiru requested review from HDinger and Kharonus December 18, 2024 22:08
@akabiru akabiru changed the title Feature/59793 create new icon for setting reminders Feature/59793 Add new icons for setting reminders op-alarm & op-alarm-set Dec 18, 2024
Copy link
Contributor

@HDinger HDinger left a comment

Choose a reason for hiding this comment

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

Nice 👍 I really like the new icons 🌟

@akabiru akabiru removed the request for review from Kharonus December 19, 2024 08:13
@akabiru akabiru merged commit 0f9edb6 into dev Dec 19, 2024
15 checks passed
@akabiru akabiru deleted the feature/59793-create-new-icon-for-setting-reminders branch December 19, 2024 08:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

2 participants