Detox icon indicating copy to clipboard operation
Detox copied to clipboard

Provide missing element visibility expectations API.

Open asafkorem opened this issue 4 years ago • 6 comments

The Issue

While extending the toBeVisible expectation API to receive an optional parameter of visibility threshold, in response to a feature request (see issue), another missing feature was brought up for discussion, alongside some questions regarding the current API.

  1. The current API for visibility expectation (toBeVisible(pct?: number)) receives only integers within the range [1, 100], and checks whether the element is visible at least with the given visibility threshold, because of underlying constraints (espresso matchers) and therefore cannot be used to check whether the view has any visibility (>0% visible), or whether is not visible at all.
  2. Negating the expectation with not, creates a confusing statement since it is quite nonintuitive what is the expected result (for example, not.toBeVisible(10), this expects the view to have less than 10% visibility).
  3. The current API name for the expectation does not implies that the pct argument is a threshold, and that the check includes the threshold as a valid visibility percentage (at-least N% visible, and not greater-than N% visible).
  4. The default value of the visibility threshold is 75, which is also very nonintuitive or makes any sense.

Suggested Solution

Add new APIs for visibility expectations, to allow any kind of visibility check with more descriptive names, and to deprecated the current method.

(1) toHaveVisibility(): checks whether the element is visible at all (the visibility percent is greater than 0%). (2) toHaveVisibilityGreaterThan(percent): checks whether the element visibility percent is greater than percent. (3) toHaveVisibilityGreaterThanOrEqualTo(percent): checks whether the element visibility percent is greater than or equal to percent.

Also, if we want to achieve an even higher level of readability in the tests, we can also provide aliases for the negations of (2) and (3):

(4) toHaveVisibilityLowerThan(percent): checks whether the element visibility percent is lower than percent. (5) toHaveVisibilityLowerThanOrEqualTo(percent): checks whether the element visibility percent is lower than or equal to percent.

asafkorem avatar Nov 01 '21 14:11 asafkorem

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you believe the issue is still relevant, please test on the latest Detox and report back.

Thank you for your contributions!

For more information on bots in this reporsitory, read this discussion.

stale[bot] avatar Dec 22 '21 19:12 stale[bot]

The issue has been closed for inactivity.

stale[bot] avatar Dec 29 '21 19:12 stale[bot]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you believe the issue is still relevant, please test on the latest Detox and report back.

Thank you for your contributions!

For more information on bots in this reporsitory, read this discussion.

stale[bot] avatar Feb 03 '22 02:02 stale[bot]

The issue has been closed for inactivity.

stale[bot] avatar Feb 10 '22 05:02 stale[bot]

@noomorph, @jonathanmos, @d4vidi – could you go over this proposal in async mode and tell what do you think, and suggest alternatives if some points don't look good enough?

Putting this into status: discussion.

noomorph avatar Jan 11 '23 14:01 noomorph

I'm inclined to say an async discussion wouldn't be very productive in this case. I think we should leave any such discussions for when things materialize such that it work on this would in fact become tangible (e.g. prioritized by us / picked up by the community) - in which case the owner could set up a discussion in sync-mode (meeting/live chat)

d4vidi avatar Jan 12 '23 10:01 d4vidi