Skip to content

Collapsible Header#191

Merged
smacker merged 1 commit into
src-d:masterfrom
smacker:collapsible_header
Mar 6, 2018
Merged

Collapsible Header#191
smacker merged 1 commit into
src-d:masterfrom
smacker:collapsible_header

Conversation

@smacker

@smacker smacker commented Mar 1, 2018

Copy link
Copy Markdown
Contributor

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

bzz commented Mar 1, 2018

Copy link
Copy Markdown
Contributor

👍

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

smacker commented Mar 1, 2018

Copy link
Copy Markdown
Contributor Author

it looks exactly as on screenshot in the issue

@dpordomingo dpordomingo left a comment

Copy link
Copy Markdown
Contributor

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
Copy Markdown
Contributor

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

@ricardobaeta

ricardobaeta commented Mar 2, 2018

Copy link
Copy Markdown
Contributor

@dpordomingo

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

Ideally it would slide down from off-canvas :)

@bzz bzz left a comment

Copy link
Copy Markdown
Contributor

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