WIP - Only handle taps on truncation token #13
Conversation
| /// Force the label to only respond to touchesEnded taps that occur on the truncation token | ||
| /// | ||
| /// This doesn't stop taps on any data elements (links, addresses and the like) | ||
| open var forceTapsOntoAttributionToken: Bool = false |
There was a problem hiding this comment.
The plan is to turn this into an enum users can set when configuring the label to decide what kind of tap handling they want.
There was a problem hiding this comment.
you mean someting like
enum TapHandling {
case wholeLabel
case tokenOnly
case none
}
?
There was a problem hiding this comment.
Something like that. We've also been talking about an option set instead internally. Either way, it'll allow the users to pick and choose what kinds of things they want to have turned on easily.
|
@pilky @ValCanBuild Here's what you were looking for as far as only handling taps on the truncation token. Instead of the boolean to enable / disable force taps on the token, I'm planning on turning it into an enum where you can pick what kind of tap handling you want, but I have a quick usability question. If you enable this mode, what happens after the user taps once and expands the text? Does it make sense to start capturing all taps after? Stop responding to taps? There's no longer truncation text to look for in the string. I have some pending changes locally I didn't include in here, so I hope it works if you want to give it a shot, but there's more work to be done to support this. |
For our case stopping responding to taps is ok as we don't allow users to shrink again. I also think this is probably the correct solution for this case as, if someone does want to change to capturing all taps afterwaards, they could change the "mode" inside the labelTappedBlock |
| return | ||
| if let touch = touches.first, let activeLink = link(at: touch.location(in: self)) { | ||
| self.activeLink = activeLink | ||
| } else if let labelTappedBlock = labelTappedBlock { |
There was a problem hiding this comment.
You probably don't want to do this as it causes the labelTappedBlock to be called twice for each tap (so using the example of toggling the number of lines, it causes it to expand on touchesBegan but then contract again on touchesEnded)
There was a problem hiding this comment.
👍 good catch, thank you
| labelTappedBlock?() | ||
| return | ||
| } else if let labelTappedBlock = labelTappedBlock { |
There was a problem hiding this comment.
I think you need to add , !forceTapsOntoAttributionToken to here, otherwise taps outside the attributed token will still go through (as the above statement will be false, but this will be true)
There was a problem hiding this comment.
Yeah, good catch. I have to rewrite this once I change the boolean as well.
|
@chansen22 I've just tried it with the 2 changes I mentioned and it seems to work pretty well. Thanks for this! |
| super.touchesBegan(touches, with: event) | ||
| } | ||
| return | ||
| if let touch = touches.first, let activeLink = link(at: touch.location(in: self)) { |
There was a problem hiding this comment.
I just noticed a potential issue here. In the case of the truncation token overlapping a link, the link will consume the click rather than the token, which seems wrong.
|
I haven't forgotten about this, coming back when I have a little more time. |
|
@chansen22 status on this one? Would be good to get in. Been using it for a while and seems stable |
|
Sorry, I haven't had any time to jump back in. I should have some coming up soon though. Since I changed how truncation works in #24. I think I should float that one down a little closer (adding tests and making sure it's all solid) and then fix this up + add in a couple improvements on how to enable / disable this setting. |
|
If you've come across any other issues while using this, feel free to add them in this thread. That'll help me finish it up more quickly |
|
Any further progress on this? 😅 |
Proof of concept to force users to tap on the truncation token to trigger the label's tap block