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

Alt image setting #13631

Merged
merged 29 commits into from
Feb 4, 2019
Merged

Alt image setting #13631

merged 29 commits into from
Feb 4, 2019

Conversation

etoledom
Copy link
Contributor

@etoledom etoledom commented Feb 1, 2019 •
edited
Loading

Description

This PR implements the Alt settings on image blocks.
The UI is temporary, so don't worry about the Textarea control being in the bottom-sheet itself, or the styles of it. We are working on it with Thomas.

Also:

  • Imported BaseControl and TextareaControl for parity with Web.
  • Removed the Done button from the bottom-sheet as requested by Thomas.
  • Small tweaks on the BottomSheet.Cell to behave better with longer texts.
  • Made the bottom-sheet avoid the keyboard on iOS. (On Android it was already working)

(Updated GIF)
alt-setting

To test:

  • Run the example project from the related gutenberg-mobile branch (Alt image setting wordpress-mobile/gutenberg-mobile#531)
  • Select an image block with image.
  • Tap on the settings button (wheel in the inline tool bar)
  • In the bottom-sheet, write something in the textinput control.
  • Check that the Alt change.
  • Close the bottom-sheet (swipe down or tap in the background)
  • Open it again.
  • Check that the old Alt value is there.

@etoledom
rnmobile: Implement image settings button using InspectorControls.Slo…
fd7c7eb 
…t pattern.
@etoledom
rnmobile: Add missing semicolon
abdfd03
@etoledom
rnmobile: Adding bottom-sheet component to mobile
1a4e35a
@etoledom
rnmobile: Styling bottom-sheet header
4352c7a
@etoledom
rnmobile: Bottom-sheet clean up
f132158
@etoledom
rnmobile: Fix lint issues on bottom-sheet related code.
7bd8a4f
@etoledom
rnmobile: Fix small lint issues
541db3b
@etoledom
rnmobile: Move inline toolbar button definition out of constant.
b8e5dcf
@etoledom
rnmobile: Remove extra white-spaces
92e5357
@etoledom
revert package-lock.json changes
b7236e0
@etoledom
Merge branch 'master' into rnmobile/bottom-sheet-component
4828348
@etoledom
rnmobile: Fix merge issue
09582b7
@etoledom
rnmobile: Imported BaseControl and TextinputControl to be used on Alt…
a1db59a 
… Image Settings
@etoledom
rnmobile: exporting component BottomSheet.Button to be used as bottom…
d335a84 
…-sheet header buttons
@etoledom
rnmobile: Adding BottomSheet.Cell component as an extraction for Bott…
67c2ffc 
…omSheet users.
@etoledom
Fix lint issues
edcd79b
@etoledom
Reverting changes to package-lock.json
fc1043a
@etoledom
Merge branch 'master' into rnmobile/bottom-sheet-component
bfd5832
@etoledom
Fix merge issues
4d7a80c
@etoledom
Merge branch 'rnmobile/bottom-sheet-component' into rnmobile/alt-imag…
05fee0c 
…e-setting
@etoledom
Remove Done button from Image settings bottom sheet
887fc58
@etoledom
Make bottom-sheet avoid being behind keyboard
a126776
@etoledom etoledom added the Mobile Web Viewport sizes for mobile and tablet devices label Feb 1, 2019
@etoledom etoledom self-assigned this Feb 1, 2019
@etoledom etoledom mentioned this pull request Feb 1, 2019
Merged
@etoledom etoledom changed the base branch from rnmobile/bottom-sheet-component to master February 1, 2019 10:31
marecar3
marecar3 approved these changes Feb 1, 2019
View reviewed changes
Copy link
Contributor

@marecar3 marecar3 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM!
update: Couldn't test on rotation change as content disappears due to other problems.

@etoledom
Copy link
Contributor Author

etoledom commented Feb 1, 2019

Thank you @marecar3 for your review!

update: Couldn't test on rotation change as content disappears due to other problems.

due to other problems, meaning that the issue not related with this PR, right?

I had to make a few changes to the Cells. To comply with Thomas design, I gave the label the possibility to be editable as a TextInput.

It should still be flexible enough for the requirements of Image Uploads, and I think that these changes won't affect it.

Still, if you could take another look it would be great. Thank you and sorry for asking review of the same PR again.

I updated the GIF in the description to show the new behavior.

image

@etoledom etoledom mentioned this pull request Feb 4, 2019
Merged
@marecar3
Copy link
Contributor

marecar3 commented Feb 4, 2019

LGTM @etoledom !

@etoledom etoledom merged commit a1b1eec into master Feb 4, 2019
@etoledom
Copy link
Contributor Author

etoledom commented Feb 4, 2019

Thank you!

daniloercoli added a commit that referenced this pull request Feb 5, 2019
@daniloercoli
Merge branch 'master' of https://github.com/WordPress/gutenberg into …
70836f1 
…rnmobile/372-use-RichText-on-Title-block

* 'master' of https://github.com/WordPress/gutenberg: (22 commits)
  Make the modal title styling consistent (#13669)
  Disable navigation block for text mode. (#12185)
  Fix: Linting problem in modal example code (#13671)
  Add myself as a code owner to the annotations (#13672)
  Add more reviewers to CODEOWNERS.md file (#13667)
  Plugin: Remove jQuery heartbeat-to-hooks proxying (#13576)
  Workflow: Add repository CODEOWNERS file (#13604)
  Add a mobile minimum size for form fields (#13639)
  Update edit-save documentation  (#13578)
  Alt image setting (#13631)
  Fix: Allow years lower than 1970 in DateTime component. (#13602)
  Using addQueryArgs to generate Manage All Reusable Blocks link (#13653)
  Bump plugin version to 5.0.0-rc.1 (#13652)
  Update lodash to 4.17.10 (#13651)
  Refreshed PR (#9469)
  Set default values of the width and height input fields according to the actual image dimensions (#7687)
  12647 fix css color picker (#12747)
  Remove "we" from messages (#13644)
  Fix: Font size picker max width on mobile (#13264)
  Fix/issue 12501 menu item aria label
  ...
@etoledom etoledom mentioned this pull request Feb 7, 2019
Closed
@youknowriad youknowriad added this to the 5.1 (Gutenberg) milestone Feb 15, 2019
This was referenced Feb 18, 2019
Merged
Merged
youknowriad pushed a commit that referenced this pull request Mar 6, 2019
@etoledom @youknowriad
Alt image setting (#13631)
3a2d9f2 
* rnmobile: Implement image settings button using InspectorControls.Slot pattern.

* rnmobile: Add missing semicolon

* rnmobile: Adding bottom-sheet component to mobile

* rnmobile: Styling bottom-sheet header

* rnmobile: Bottom-sheet clean up

* rnmobile: Fix lint issues on bottom-sheet related code.

* rnmobile: Fix small lint issues

* rnmobile: Move inline toolbar button definition out of constant.

* rnmobile: Remove extra white-spaces

* revert package-lock.json changes

* rnmobile: Fix merge issue

* rnmobile: Imported BaseControl and TextinputControl to be used on Alt Image Settings

* rnmobile: exporting component BottomSheet.Button to be used as bottom-sheet header buttons

* rnmobile: Adding BottomSheet.Cell component as an extraction for BottomSheet users.

* Fix lint issues

* Reverting changes to package-lock.json

* Fix merge issues

* Remove Done button from Image settings bottom sheet

* Make bottom-sheet avoid being behind keyboard

* Fix lint issues

* Making BottomSheet.Cell value editable as textinput.

* Remove unnecesary onPress prop from Alt cell.

* Fix lint issues
youknowriad pushed a commit that referenced this pull request Mar 6, 2019
@etoledom @youknowriad
Alt image setting (#13631)
813534a 
* rnmobile: Implement image settings button using InspectorControls.Slot pattern.

* rnmobile: Add missing semicolon

* rnmobile: Adding bottom-sheet component to mobile

* rnmobile: Styling bottom-sheet header

* rnmobile: Bottom-sheet clean up

* rnmobile: Fix lint issues on bottom-sheet related code.

* rnmobile: Fix small lint issues

* rnmobile: Move inline toolbar button definition out of constant.

* rnmobile: Remove extra white-spaces

* revert package-lock.json changes

* rnmobile: Fix merge issue

* rnmobile: Imported BaseControl and TextinputControl to be used on Alt Image Settings

* rnmobile: exporting component BottomSheet.Button to be used as bottom-sheet header buttons

* rnmobile: Adding BottomSheet.Cell component as an extraction for BottomSheet users.

* Fix lint issues

* Reverting changes to package-lock.json

* Fix merge issues

* Remove Done button from Image settings bottom sheet

* Make bottom-sheet avoid being behind keyboard

* Fix lint issues

* Making BottomSheet.Cell value editable as textinput.

* Remove unnecesary onPress prop from Alt cell.

* Fix lint issues
@etoledom etoledom deleted the rnmobile/alt-image-setting branch July 1, 2019 10:28
@gziolo gziolo added Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) and removed Mobile Web Viewport sizes for mobile and tablet devices labels Oct 10, 2019
This was referenced Jul 16, 2024
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marecar3 marecar3 marecar3 approved these changes

Assignees

@etoledom etoledom

Labels
Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change)
Projects
None yet
Milestone
5.1 (Gutenberg)
Development

Successfully merging this pull request may close these issues.
4 participants
@etoledom @marecar3 @youknowriad @gziolo