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.

Sign up for GitHub

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

Jump to bottom

Mobile: Link image setting #13654

Merged
merged 40 commits into from
Feb 5, 2019
Merged

Mobile: Link image setting #13654

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
fd7c7eb
rnmobile: Implement image settings button using InspectorControls.Slo…
etoledom Jan 30, 2019
abdfd03
rnmobile: Add missing semicolon
etoledom Jan 30, 2019
1a4e35a
rnmobile: Adding bottom-sheet component to mobile
etoledom Jan 30, 2019
4352c7a
rnmobile: Styling bottom-sheet header
etoledom Jan 30, 2019
f132158
rnmobile: Bottom-sheet clean up
etoledom Jan 30, 2019
7bd8a4f
rnmobile: Fix lint issues on bottom-sheet related code.
etoledom Jan 30, 2019
541db3b
rnmobile: Fix small lint issues
etoledom Jan 31, 2019
b8e5dcf
rnmobile: Move inline toolbar button definition out of constant.
etoledom Jan 31, 2019
92e5357
rnmobile: Remove extra white-spaces
etoledom Jan 31, 2019
b7236e0
revert package-lock.json changes
etoledom Jan 31, 2019
4828348
Merge branch 'master' into rnmobile/bottom-sheet-component
etoledom Jan 31, 2019
09582b7
rnmobile: Fix merge issue
etoledom Jan 31, 2019
a1db59a
rnmobile: Imported BaseControl and TextinputControl to be used on Alt…
etoledom Jan 31, 2019
d335a84
rnmobile: exporting component BottomSheet.Button to be used as bottom…
etoledom Jan 31, 2019
67c2ffc
rnmobile: Adding BottomSheet.Cell component as an extraction for Bott…
etoledom Jan 31, 2019
edcd79b
Fix lint issues
etoledom Jan 31, 2019
fc1043a
Reverting changes to package-lock.json
etoledom Jan 31, 2019
bfd5832
Merge branch 'master' into rnmobile/bottom-sheet-component
etoledom Jan 31, 2019
4d7a80c
Fix merge issues
etoledom Jan 31, 2019
05fee0c
Merge branch 'rnmobile/bottom-sheet-component' into rnmobile/alt-imag…
etoledom Jan 31, 2019
887fc58
Remove Done button from Image settings bottom sheet
etoledom Jan 31, 2019
a126776
Make bottom-sheet avoid being behind keyboard
etoledom Jan 31, 2019
feab6e0
Merge branch 'master' into rnmobile/alt-image-setting
etoledom Feb 1, 2019
30ed40c
Fix lint issues
etoledom Feb 1, 2019
1e45675
Merge branch 'master' into rnmobile/alt-image-setting
etoledom Feb 1, 2019
5c6cd81
Making BottomSheet.Cell value editable as textinput.
etoledom Feb 1, 2019
a3f44ba
Remove unnecesary onPress prop from Alt cell.
etoledom Feb 1, 2019
290d4fc
Image Settings: Added Link To setting to bottom-sheet
etoledom Feb 4, 2019
dab0fd2
BottomSheet desing details
etoledom Feb 4, 2019
9098808
Fix bottom-sheet cell value growing larger than container
etoledom Feb 4, 2019
4643b01
Bottom-sheet: Fix bottom safe-area inset with keyboard showing
etoledom Feb 4, 2019
7e391ba
Fix lint issues
etoledom Feb 4, 2019
6d60a5f
Merge branch 'master' into rnmobile/alt-image-setting
etoledom Feb 4, 2019
890404b
Merge branch 'rnmobile/alt-image-setting' into rnmobile/link-image-se…
etoledom Feb 4, 2019
a6efb9a
Adding textinput props to bottom-sheet cell value.
etoledom Feb 4, 2019
b0fc616
Removing autocorrect from link-setting text input.
etoledom Feb 4, 2019
6d525d8
Fix lint issues
etoledom Feb 4, 2019
33ad7c5
Merge branch 'master' into rnmobile/link-image-setting
etoledom Feb 4, 2019
0dee9bf
Fixing label texts on Image Settings bottom-sheet
etoledom Feb 4, 2019
5849829
Mobile ImageEdit color using global definition.
etoledom Feb 5, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 22 additions & 3 deletions packages/block-library/src/image/edit.native.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ const MEDIA_UPLOAD_STATE_SUCCEEDED = 2;
const MEDIA_UPLOAD_STATE_FAILED = 3;
const MEDIA_UPLOAD_STATE_RESET = 4;

const LINK_DESTINATION_CUSTOM = 'custom';

export default class ImageEdit extends React.Component {
constructor( props ) {
super( props );
Expand All @@ -45,6 +47,7 @@ export default class ImageEdit extends React.Component {
this.finishMediaUploadWithSuccess = this.finishMediaUploadWithSuccess.bind( this );
this.finishMediaUploadWithFailure = this.finishMediaUploadWithFailure.bind( this );
this.updateAlt = this.updateAlt.bind( this );
this.onSetLinkDestination = this.onSetLinkDestination.bind( this );
this.onImagePressed = this.onImagePressed.bind( this );
}

Expand Down Expand Up @@ -133,9 +136,16 @@ export default class ImageEdit extends React.Component {
this.props.setAttributes( { alt: newAlt } );
}

onSetLinkDestination( href ) {
this.props.setAttributes( {
linkDestination: LINK_DESTINATION_CUSTOM,
href,
} );
Copy link
Contributor

Choose a reason for hiding this comment

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

so I didn't quite get how to clear the linkDestination here. is it sth to be implemented in the future? Would it be good to clear the LINK_DESTINATION_CUSTOM if the field is empty?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is interesting. We are not implementing the full links feature in mobile yet, but it would be good if they don't break each other. 🤔

For a proper comparison of LINK_DESTINATION_MEDIA we need the image prop with the source_url information. Not sure if props.attributes.url is enough. In the next PR I'm working on getting that image media.

I'll also talk with Thomas about this to know what would be the best approach.
Thanks for noticing this!

}

render() {
const { attributes, isSelected, setAttributes } = this.props;
const { url, caption, height, width, alt } = attributes;
const { url, caption, height, width, alt, href } = attributes;

const onMediaLibraryButtonPressed = () => {
requestMediaPickFromMediaLibrary( ( mediaId, mediaUrl ) => {
Expand Down Expand Up @@ -197,6 +207,15 @@ export default class ImageEdit extends React.Component {
onClose={ onImageSettingsClose }
hideHeader
>
<BottomSheet.Cell
icon={ 'admin-links' }
label={ __( 'Link To' ) }
value={ href || '' }
valuePlaceholder={ __( 'Add URL' ) }
onChangeValue={ this.onSetLinkDestination }
autoCapitalize="none"
autoCorrect={ false }
/>
<BottomSheet.Cell
icon={ 'editor-textcolor' }
label={ __( 'Alt Text' ) }
Expand All @@ -205,8 +224,8 @@ export default class ImageEdit extends React.Component {
onChangeValue={ this.updateAlt }
/>
<BottomSheet.Cell
label={ __( 'Reset to original' ) }
labelStyle={ { color: 'red' } }
label={ __( 'Reset to Original' ) }
labelStyle={ styles.resetSettingsButton }
drawSeparator={ false }
onPress={ () => {} }
/>
Expand Down
4 changes: 4 additions & 0 deletions packages/block-library/src/image/styles.native.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,7 @@
flex-direction: column;
align-items: center;
}

.resetSettingsButton {
color: $alert-red;
}
12 changes: 8 additions & 4 deletions packages/editor/src/components/mobile/bottom-sheet/cell.native.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,28 +24,30 @@ export default function Cell( props ) {
labelStyle = {},
valueStyle = {},
onChangeValue,
...valueProps
} = props;

const showValue = value !== undefined;
const isValueEditable = onChangeValue !== undefined;
const defaultLabelStyle = showValue ? styles.cellLabel : styles.cellLabelCentered;
const separatorStyle = showValue ? styles.cellSeparator : styles.separator;
let valueTextInput;

const onCellPress = () => {
if ( isValueEditable ) {
valueTextInput.focus();
} else {
} else if ( onPress !== undefined ) {
onPress();
}
};

return (
<TouchableOpacity onPress={ onCellPress }>
<TouchableOpacity onPress={ onCellPress } >
<View style={ styles.cellContainer }>
<View style={ styles.cellRowContainer }>
{ icon && (
<View style={ styles.cellRowContainer }>
<Dashicon icon={ icon } size={ 30 } />
<Dashicon icon={ icon } size={ 24 } />
<View style={ { width: 12 } } />
</View>
) }
Expand All @@ -60,13 +62,15 @@ export default function Cell( props ) {
style={ { ...styles.cellValue, ...valueStyle } }
value={ value }
placeholder={ valuePlaceholder }
placeholderTextColor={ '#87a6bc' }
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like this color doesn't have a name in gutenberg repo. However, we can maybe put it into an scss and give it a name?(like placeholderGray, I don't know, you name it) that way it might be easier to import the scss from somewhere else and reuse in the future? Although, I don't have a strong opinion on this, so I am totally fine with the way you are willing to choose.

onChangeText={ onChangeValue }
editable={ isValueEditable }
{ ...valueProps }
/>
) }
</View>
{ drawSeparator && (
<View style={ styles.separator } />
<View style={ separatorStyle } />
) }
</TouchableOpacity>
);
Expand Down
1 change: 1 addition & 0 deletions packages/editor/src/components/mobile/bottom-sheet/index.native.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ class BottomSheet extends Component {
<KeyboardAvoidingView
behavior={ Platform.OS === 'ios' && 'padding' }
style={ { ...styles.content, borderColor: 'rgba(0, 0, 0, 0.1)' } }
keyboardVerticalOffset={ -this.state.safeAreaBottomInset }
>
<View style={ styles.dragIndicator } />
{ hideHeader || (
Expand Down
34 changes: 18 additions & 16 deletions packages/editor/src/components/mobile/bottom-sheet/styles.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
background-color: $light-gray-400;
height: 4px;
width: 10%;
top: -12px;
margin: auto;
border-radius: 2px;
}
Expand All @@ -18,12 +17,12 @@
background-color: $light-gray-400;
height: 1px;
width: 100%;
margin-bottom: 14px;
}

.content {
padding: 6px 16px 0 16px;
background-color: $white;
padding: 18px 10px 5px 10px;
justify-content: center;
border-top-right-radius: 8px;
border-top-left-radius: 8px;
}
Expand Down Expand Up @@ -59,41 +58,44 @@

// Cell

//Bottom Sheet

.cellContainer {
flex-direction: row;
min-height: 50;
justify-content: space-between;
margin-left: 12;
margin-right: 12;
min-height: 48;
align-items: center;
}

.cellSeparator {
background-color: $light-gray-400;
height: 1px;
width: 100%;
margin-left: 36px;
}

.cellRowContainer {
flex-direction: row;
align-items: center;
}

.cellIcon {
padding-right: 30;
padding-right: 0;
}

.cellLabel {
font-size: 18px;
color: #000;
font-size: 17px;
color: #2e4453;
Copy link
Contributor

Choose a reason for hiding this comment

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

is it possible that we use $gray-dark instead of #2e4453?
and also for the below 2 usages?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

$gray-dark is defined in gutenberg-mobile, so we don't have access to it on gutenberg.
It might be a good idea to move all those color definitions to a .mobile.scss in gutenberg, but I'd rather not do it in this PR.

margin-right: 12px;
}

.cellLabelCentered {
font-size: 18px;
color: #000;
font-size: 17px;
color: #2e4453;
flex: 1;
text-align: center;
}

.cellValue {
font-size: 18px;
color: $dark-gray-400;
font-size: 17px;
color: #2e4453;
text-align: right;
flex: 1;
}