8000 Collapsible Header by smacker · Pull Request #191 · src-d/code-annotation · GitHub
[go: up one dir, main page]

Skip to content

Conversation

@smacker
Copy link
Contributor
@smacker smacker commented Mar 1, 2018

Fixes: #54

Normal state:
image

When you hover the header, the other header appears above the initial one:
image

Signed-off-by: Maxim Sukharev <maxim@sourced.tech>
@smacker smacker requested a review from bzz March 1, 2018 15:51
@bzz
Copy link
Contributor
bzz commented Mar 1, 2018

👍

but after

it's kind-of starts to feel a bit repetitive 😆 but still, in my experience, having a screenshot for UI changes speeds up the review.

@bzz bzz requested a review from carlosms March 1, 2018 16:59
@smacker
Copy link
Contributor Author
smacker commented Mar 1, 2018

it looks exactly as on screenshot in the issue

Copy link
Contributor
@dpordomingo dpordomingo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM,
many thanks @smacker for removing that much of duplicated code 💯 👍

@dpordomingo
Copy link
Contributor

@bzz I updated the issue desc with two screenshots comparing the two states, so then the PR can be easily reviewed

@ricardobaeta
Copy link
Contributor
ricardobaeta commented Mar 2, 2018

@dpordomingo

When you hover the header, the other header appears above the initial one:

Ideally it would slide down from off-canvas :)

Copy link
Contributor
@bzz bzz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great to me

@smacker smacker merged commit 0aac358 into src-d:master Mar 6, 2018
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.

4 participants

0