Skip to content
This repository has been archived by the owner on Jun 5, 2020. It is now read-only.

Fixed action preview header on the top #46

Merged
merged 2 commits into from Nov 18, 2016
Merged

Conversation

jhen0409
Copy link
Contributor

Currently we cannot set correctly height: 100% to custom tab content height:

2016-10-15 6 13 03

Set flex: 1 to actionPreview style will succeed:

2016-10-15 6 13 54

@jhen0409 jhen0409 changed the title Fix tab content height Fixed action preview header on the top Oct 16, 2016
@jhen0409
Copy link
Contributor Author

I'm changed this PR, just use flex layout to fixed action preview header on the top:

2016-10-16 13_33_26

/cc @zalmoxisus

'align-items': 'center',
'border-bottom-width': '1px',
'border-bottom-style': 'solid',
'min-height': '30px',
'min-height': '40px',
Copy link
Owner

Choose a reason for hiding this comment

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

how about flex: 0 0 40px instead of min-height and flex-basis?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, that will be better. it's done, I also removed margin-bottom of tabSelector.

Copy link
Collaborator

Choose a reason for hiding this comment

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

What's the reason of increasing the height? I'm getting an unexpected extra space with that:
image

Which is fixed by changing to

-flex: '0 0 40px',
+flex: '0 0 30px',

Copy link
Contributor Author

@jhen0409 jhen0409 Oct 20, 2016

Choose a reason for hiding this comment

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

@zalmoxisus because I used min-height and flex-basis before, 30px will get unexpected result, but we should keep 30px for change to flex: '0 0 px', it's my fault, I'll update it.

@jhen0409 jhen0409 force-pushed the patch-1 branch 2 times, most recently from 698ee5e to 3dee9d1 Compare October 16, 2016 11:21
@zalmoxisus
Copy link
Collaborator

zalmoxisus commented Oct 19, 2016

I might be missing something, but using this patch and adding

  <div style={{
    display: 'flex',
    alignItems: 'center',
    justifyContent: 'center',
    width: '100%',
+    height: '100%',
+    backgroundColor: '#ccc',
    minHeight: '20rem'
  }}>

to the demo, I'm getting the same issue in the first case as reported:

image

@jhen0409
Copy link
Contributor Author

@zalmoxisus, I missing add flex: 1 to actionPreview, it should work for now.

@zalmoxisus
Copy link
Collaborator

@jhen0409, it did the trick, thanks! Not sure you noticed also my comment on the outdated diff.

@jhen0409
Copy link
Contributor Author

jhen0409 commented Nov 2, 2016

@alexkuz @zalmoxisus any update on this? :)

@alexkuz
Copy link
Owner

alexkuz commented Nov 2, 2016

@jhen0409 I'll try to take a closer look on this (and other stuff I'm shamelessly ignoring lately) this or next weekend. Sorry for making you wait!

@alexkuz alexkuz merged commit a46147c into alexkuz:master Nov 18, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants