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

Remove the view switcher component #513

Merged
merged 1 commit into from Aug 1, 2017
Merged

Conversation

seanh
Copy link
Contributor

@seanh seanh commented 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 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

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
seanh added a commit to hypothesis/h that referenced this pull request Jul 31, 2017
@codecov
Copy link

codecov bot commented Jul 31, 2017

Codecov Report

Merging #513 into master will increase coverage by 0.05%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #513      +/-   ##
==========================================
+ Coverage   90.76%   90.82%   +0.05%     
==========================================
  Files         135      134       -1     
  Lines        5394     5371      -23     
  Branches      942      936       -6     
==========================================
- Hits         4896     4878      -18     
+ Misses        498      493       -5
Impacted Files Coverage Δ
src/sidebar/components/sidebar-content.js 89.67% <ø> (ø) ⬆️

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 1950585...9106106. Read the comment docs.

@robertknight
Copy link
Member

LGTM

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