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

fix(feedback): more accessibility fixes #4727

Open
wants to merge 8 commits into
base: armcknight/armcknight/feat(feedback)/form-controller-refactor
Choose a base branch
from

Conversation

armcknight
Copy link
Member

this has a couple simple fixes that i found i needed to make while refactoring in #4726, and wanted to keep them separate

#skip-changelog

@armcknight armcknight changed the title log warning fix(feedback): more accessibility fixes Jan 20, 2025
Copy link

github-actions bot commented Jan 20, 2025

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1225.69 ms 1239.69 ms 13.99 ms
Size 22.31 KiB 777.51 KiB 755.19 KiB

Baseline results on branch: armcknight/armcknight/feat(feedback)/form-controller-refactor

Startup times

Revision Plain With Sentry Diff
484eb3c 1222.22 ms 1234.35 ms 12.12 ms

App size

Revision Plain With Sentry Diff
484eb3c 22.31 KiB 776.35 KiB 754.04 KiB

Previous results on branch: armcknight/feat(feedback)/accessibility-fixes

Startup times

Revision Plain With Sentry Diff
ca04f74 1229.92 ms 1248.83 ms 18.91 ms

App size

Revision Plain With Sentry Diff
ca04f74 22.31 KiB 776.63 KiB 754.31 KiB

Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

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

LGTM, just CI is a bit complaining.

@@ -80,7 +80,10 @@ extension SentryUserFeedbackFormController {
}

func showedKeyboard(note: Notification) {
guard let keyboardValue = note.userInfo?[UIResponder.keyboardFrameEndUserInfoKey] as? NSValue else { return }
guard let keyboardValue = note.userInfo?[UIResponder.keyboardFrameEndUserInfoKey] as? NSValue else {
SentryLog.warning("Received a keyboard display notification with no frame information.")
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the warning 👍

Copy link
Contributor

@philprime philprime left a comment

Choose a reason for hiding this comment

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

LGTM

…factor' into armcknight/feat(feedback)/accessibility-fixes
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.

3 participants