Add a minimum time criterion #4
Conversation
* Add an optional minimum time criterion between the first active positive event and the current time
joeljfischer
force-pushed
the
feature/issue-2-minimum-time-before-request-review
branch
from
2c184f8
to
3ffc597
Compare
There was a problem hiding this comment.
Thank you for suggesting this feature and for the example implementation. 🙏
I do understand your point that in some apps you might not want to request a review on the first day. I do not think that this is something that should be enabled by default or even recommended in the library though – from my personal experience and what I read elsewhere it's actually best practice to already ask for review on first app use given that some positive actions occurred. But we could certainly cover your use case to add some more flexibility to the library.
To keep the library and docs simple, I would actually prefer to give this option its own property though. I'm also not sure about the naming being minimumTimeBeforeRequest – it's not very clear to me what it refers to. I initially thought what you're asking for is a delay right after requestReview is called until the prompt is presented. So I would prefer a name along the lines of avoidRequestsOnFirstDay to be more clear. Maybe we don't even need any other options than to provide a Bool?
Another question is: Should the delay really relate to the first event in the list? We could easily add another storage for the first time a call was made, then it could be related to the first day app is used.
|
Regarding the time frame, I would prefer something more flexible like what I currently have implemented. In my app I have a delay of a week, for example. I understand the current naming being unclear. If you wanted to go the direction of using a As far as the first event in the list vs. the first app open, I think the first event in the list makes more sense. What I was trying to avoid was a situation where the user opens the app, then doesn't use it for a month, then opens it again and starts using it. I'd prefer to see the later app open as the new "first time it's used". |
Fixes #2
Proposed Changes