getsentry
User Feedback UI widget
Description
Build the interface widget an end-user would tap to present the feedback capture form
IMO, a UI widget is overkill. I believe a public API to open the feedback form is enough. The user can be responsible for creating the button, link, or action that opens the form, as each app has its own visual identity.
@brustolin, I think that's the main point of this new feature. If I'm not mistaken, people on the web love it, but I'm also unsure if folks on native mobile apps would use it. @bruno-garcia or @armcknight, maybe you can elaborate on why it's worth the effort to build a UI widget.
@philipphofmann I believe the main point is the form screen isn't it?
Yes, but I guess many iOS developers don't want a Sentry-flavored form screen but rather design their own. That was the main reason why we didn't add it in the past.
There will be a way for customers to BYOB (bring your own button) and hook it into the integration. They would be completely responsible for displaying that button, we just use the touch event to display our form.
I hadn't yet considered an option where we take a button from them, but manage displaying it. That could be another nice option, so they get their branding, but don't have to worry about the mechanics.
In any case, it's not a requirement to use the widget. It's another option, there may be people out there that feel differently than you about this. It doesn't make sense to withhold the option from them just because someone here doesn't like it. Ideally we can attach analytics to it and we can see how many people use each method in practice.
Current state of the widget:
using an all-default config (except positioning, which I used to nudge it out of the way of the tab bar) and with some [extreme] overrides:
| default | customized |
|---|---|
So there's a little more work to do to correctly size/align the megaphone with edge case-y font sizes (which will be a concern with accessibility settings).
Working to get the icon and text perfectly vertically centered in the button, but it's tough because different font faces have different amounts of space inside the text bounding box, so i need to find the right combination of constraints that work generally across them all. See for instance ChalkBoardSE-Regular vs HelveticaNeue:
ChalkBoardSE-Regular |
HelveticaNeue |
|---|---|
I've got the font-lozenge centering complete, although it still results in slight variation in the top/bottom padding due to the differences in fonts' cap height and ascender values and how that affects how text is drawn inside a UILabel:
| Font face | HelveticaNeue | ChalkboardSE-Regular |
|---|---|---|
| Demo |
I think it's not worth pursuing equalizing that between fonts at this point, as people will pick a font they need, and then make the layout horizontal/vertical adjustments, and be done with it.
And also scaling the megaphone with the size of the font:
| Font size | 16 | 32 | 64 |
|---|---|---|---|
| Demo |
I got stuck for a while on the Damascus font. It has some inverted measurements that make it very difficult to generalize the approach. After looking some more, I found out that that font is actually meant to draw Arabic characters, and when it renders Latin characters is when it gets wonky. Looks fine with Arabic:
| Latin | Arabic |
|---|---|
I think it's fine to avoid dealing with that edge case. If someone complains, we should probably just tell them "don't do that" but we can always try to fix it at that point.
I think it's fine to avoid dealing with that edge case. If someone complains, we should probably just tell them "don't do that" but we can always try to fix it at that point.
If think it's OK to skip some edge cases in the first iteration. If people adopt the feature, use it, and complain, we can invest in fixing more edge cases.
A couple other things I'll need to check:
- accessibility font sizes. i think there's another API i need to use so that our text actually scales with it.
- other interesting fonts like Damascus. I've been told Devenagari and Hebrew both have possibly inverted baselines/ascenders or other interesting vertical characteristics.
- we'll want the megaphone to flip and be on the other side for right-to-left languages like Hebrew, Arabic etc
Question: do we want the vertical padding to scale with the font size, or keep it fixed? With fixed spacing, it looks weirder the smaller the font gets, because the negative space gets larger. But with scaling, the larger the font, the more negative space. Maybe we scale it below the system default, and keep it fixed above?
| Font size | 8 | 14 (system default) | 40 |
|---|---|---|---|
| Fixed | |||
| Scaling |
Some right-to-left testing, to make sure the megaphone appears on the leading side, and faces the right way:
| Urdu | Hebrew | Arabic |
|---|---|---|
Guess there's still an issue with fonts that have inverted ascenders that will need to be addressed, judging from the arabic example (using Damascus font)
Also, I would want the button on the other side of the screen. I previously had a UIDirectionalRectEdge property in SentryUserFeedbackWidgetConfiguration but dialed it back to just UIRectEdge since the former is only available starting in iOS 13. The issue here is that the swift definition won't export individual @available properties to the generated ObjC interface :/
FYI I found that some of the languages/locales to test this didn't work until I installed the following keyboards:
Then I was able to test by changing the plist value
with one of the ones noted in the SDK init in iOS-Swift: https://github.com/getsentry/sentry-cocoa/pull/4364/files#diff-489e20902b3b8cb67d406ba71c59e123294eead63461fd69a475859ad75a6309R177-R187 and https://github.com/getsentry/sentry-cocoa/pull/4364/files#diff-489e20902b3b8cb67d406ba71c59e123294eead63461fd69a475859ad75a6309R209-R219
Working with dynamic type size with the stock button (and you can see the one button we have in the sample app that also supports it 😆 ):
| settings | app |
|---|---|
We've decided to only offer this feature for iOS 13+ for now. If it becomes an issue we can always build in backwards compatibility.
After deciding to move to iOS 13+, fixing the alignment of the button in rtl is trivial:
Think I got the vertical centering right for Damascus with arabic:
| 60pt | 40pt | 14pt (default) | 8pt |
|---|---|---|---|