Skip to content

BottomSheet centered with maximum width.#600

Merged
etoledom merged 11 commits intodevelopfrom
issue/bottom-sheet-width-centered
Feb 19, 2019
Merged

BottomSheet centered with maximum width.#600
etoledom merged 11 commits intodevelopfrom
issue/bottom-sheet-width-centered

Conversation

@etoledom
Copy link
Copy Markdown
Contributor

Fixes: #572

centered-bottom-sheet
simulator screen shot - ipad pro 9 7-inch - 2019-02-14 at 16 45 25

This PR updates the gutenberg ref to test WordPress/gutenberg#13882

Note: Don't worry about the separators going out of the bottom sheet. That is resolved here: WordPress/gutenberg#13855

To test:
Please refer to that PR for testing.

cc @iamthomasbishop

@etoledom etoledom added the [Type] Enhancement Improves a current area of the editor label Feb 14, 2019
@etoledom etoledom added this to the Beta milestone Feb 14, 2019
@etoledom etoledom self-assigned this Feb 14, 2019
@etoledom etoledom changed the title Update gutenberg ref BottomSheet centered with maximum width. Feb 14, 2019
@iamthomasbishop
Copy link
Copy Markdown
Contributor

This is looking great, @etoledom! Assuming the border-width issue and scrollable issues are being addressed separately, this one looks good to go.

@etoledom etoledom requested a review from pinarol February 19, 2019 09:01
Copy link
Copy Markdown
Contributor

@pinarol pinarol left a comment

Choose a reason for hiding this comment

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

Thanks for handling the inserter ui refactoring also! This looks great. Tested both with inserter ui and image settings ui, works pretty good on both safe area and normal screens on portrait/landscape modes. Left just a few tiny comments about deleting the unused items.

import { getBlockTypes } from '@wordpress/blocks';

type PropsType = {
style?: StyleSheet,
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 like we don't need safeAreaBottomInset anymore

and we can also get rid of this at the bottom of the page:

BlockPicker.defaultProps = {
safeAreaBottomInset: 0,
};

render() {
const numberOfColumns = this.calculateNumberOfColumns();
const paddingBottom = this.paddingBottom();
const bottomPadding = this.props.addExtraBottomPadding && styles.contentBottomPadding;
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.

we can add addExtraBottomPadding to PropsType

@etoledom
Copy link
Copy Markdown
Contributor Author

Since this PR was so small, I took the opportunity to replace the BlockPicker Modal with the BottomSheet component, as a follow up of #608

cc @iamthomasbishop

picker

@etoledom etoledom merged commit 2a715ce into develop Feb 19, 2019
@etoledom etoledom deleted the issue/bottom-sheet-width-centered branch February 19, 2019 10:20
@etoledom
Copy link
Copy Markdown
Contributor Author

Thank you!

@iamthomasbishop
Copy link
Copy Markdown
Contributor

Looks great! One minor thing, but we can address it separately – the left/right sheet margins on the smaller portrait Android example are a bit narrow. Based on the screenshots I'm guessing we're using a static width for each block type which is fine for now, but we might want to adjust it to use percentages at some point to accommodate narrow screens.

@etoledom
Copy link
Copy Markdown
Contributor Author

etoledom commented Feb 19, 2019

I'm guessing we're using a static width for each block

Yes, width is static. If the screen is thin enough, the number of columns will be reduced to 2.
Of course, we can iterate and improve this :)

@iamthomasbishop
Copy link
Copy Markdown
Contributor

Awesome, thanks for the confirmation!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Type] Enhancement Improves a current area of the editor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement BottomSheet max width and centered.

3 participants