Skip to content

Conversation

@efekrskl
Copy link
Contributor

  • Fixes notification adapter not responding to close events (e.g pressing X or ESC button)
  • Adjusts notification adapter modal so it matches Provider modal style wise (button ordering, styles, modal size etc.)
  • Changes Blacklist icon (right now Blacklist and Notification Adapters have same icons)
  • Fixes minor typos

title="Adding a new Notification Adapter"
visible={visible}
style={{ width: '95%' }}
style={{ width: '50rem' }}
Copy link
Owner

Choose a reason for hiding this comment

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

This will break using this on mobile...

Copy link
Owner

@orangecoding orangecoding Nov 16, 2025

Choose a reason for hiding this comment

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

@efekrskl can you check my comments? ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@orangecoding hey, yup, great points on both PRs. I'm a bit busy, but I'll get back to these.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so I just got some time to get back to this:

you are right, this definitely breaks in mobile. however, this page already doesn't work on mobile, as the other modal already uses 50rem (and I copied this value from there). There are other minor issues like some buttons overflowing.

I pushed a commit checking for screen width so for larger screens we can have a smaller modal, and for smaller ones we could use 95%. This is just an idea though, lmk what you think :)

@orangecoding orangecoding merged commit 3eb3f6e into orangecoding:master Nov 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants