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 view switcher flash on iOS #482

Merged
merged 1 commit into from Jul 4, 2017

Conversation

seanh
Copy link
Contributor

@seanh seanh commented Jul 3, 2017

The view switcher tabs at the top of the sidebar would flash grey for a
moment after you clicked one. This removes the flash.

This webkit "tap highlight" is actually an iOS Safari usability feature,
it indicates to the user that their tap is being successfully recognized, and
indicates which element they're tapping on. But I don't think it looks
good with these tabs of ours, and it seems to interact badly with the
background colour animated transition we're using (it seems to do the
animated transition first and then flash the tap highlight colour for
a second).

We're providing this indication by changing the colour of the tab
anyway, so disable it.

The view switcher tabs at the top of the sidebar would flash grey for a
moment after you clicked one. This removes the flash.

This webkit "tap highlight" is actually an iOS Safari usability feature,
it indicates to the user that their tap is being successfully recognized, and
indicates which element they're tapping on. But I don't think it looks
good with these tabs of ours, and it seems to interact badly with the
background colour animated transition we're using (it seems to do the
animated transition first and _then_ flash the tap highlight colour for
a second).

We're providing this indication by changing the colour of the tab
anyway, so disable it.
@codecov
Copy link

codecov bot commented Jul 3, 2017

Codecov Report

Merging #482 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #482   +/-   ##
=======================================
  Coverage   89.84%   89.84%           
=======================================
  Files         133      133           
  Lines        5151     5151           
  Branches      881      881           
=======================================
  Hits         4628     4628           
  Misses        523      523

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ee080fe...450beed. Read the comment docs.

@robertknight
Copy link
Member

LGTM.

@robertknight robertknight merged commit 2797616 into master Jul 4, 2017
@robertknight robertknight deleted the fix-view-switcher-flicker-on-ios branch July 4, 2017 10:37
seanh added a commit that referenced this pull request Jul 31, 2017
Remove the (feature-flagged) view switcher component.

We missed the chance to strike while the iron was hot on this one -
neither Dawa or I have time anymore to work on further design
iterations. I don't want to have dead code living behind a feature flag
so I'm removing it. The code will still be in git if we ever want to
revive it. Perhaps these tabs can be re-designed as part of a larger
re-design of the sidebar in the future.

I'll leave the GitHub issue about the usability issues with the existing
selection tabs open: hypothesis/product-backlog#327

For the record the main remaining issues with this were:

1. An undesirable visual popping happens when the view switcher loads on
pages that have a lot of annotations. This also happens with the
existing selection tabs but is arguably more visible with the view
switcher.

2. When there are only two tabs (no orphans) the visual design of the
tabs doesn't make it as immediately obvious which is the currently
selected tab. The other tab may be mistaken for the selected one.

The TODO list was here:

hypothesis/product-backlog#327 (comment)

The pull requests for this view switcher were:

#429
#481
#482
hypothesis/product-backlog#327
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.

None yet

2 participants