-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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.
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
Fixes position of upload svg on mobile #2693
Conversation
This works for any 'not large' device.
Codecov Report
@@ Coverage Diff @@
## master #2693 +/- ##
==========================================
+ Coverage 33.69% 33.93% +0.24%
==========================================
Files 185 185
Lines 5594 5743 +149
Branches 976 1020 +44
==========================================
+ Hits 1885 1949 +64
- Misses 3141 3203 +62
- Partials 568 591 +23
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is #1745 the right issue to be closing? I'm not sure what to be testing here.
editor/header/tools/style.scss
Outdated
@@ -106,6 +106,13 @@ | |||
margin-bottom: 0; | |||
} | |||
|
|||
@media ( max-width: $break-large ) { | |||
.wp-core-ui .button.wp-block-image__upload-button.button.button-large { | |||
padding: 6px 21px 6px 6px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mixed spaces / tabs here.
@karmatosed, it looks like wrong issue is referenced here. Can you provide some testing instructions here? |
editor/header/tools/style.scss
Outdated
@@ -106,6 +106,13 @@ | |||
margin-bottom: 0; | |||
} | |||
|
|||
@media ( max-width: $break-large ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use @include break-large()
on the upload button component?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated to make it mobile first and moved styling to the component's style file.
I have updated with the issue it should have referenced, my apologies it was relating to a different issue. #2671 |
@mtias can you validate my style changes? Just saying, I'm not an expert in this field :) |
@gziolo seems good, though you might also have to do for galleries (that's why I was hoping it could be done for https://github.com/WordPress/gutenberg/tree/master/blocks/media-upload-button directly). |
It makes sense, I will update 👍 |
caac15d
to
6bde531
Compare
@@ -0,0 +1,11 @@ | |||
.components-form-file-upload { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The file should be style.scss
not editor
; editor.scss
is used to differentiate editor styles for blocks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. It makes sense, but it's a bit confusing. I assumed that it needs to be editor view as this is where it only applies :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The editor / style split is only for blocks, as we need to load those on the front end, and we don't want to pull block styles that are meant for the editor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making it clear :) It's now updated to match the convention.
6bde531
to
b2d861b
Compare
🚢 it |
This works for any 'not large' device.
Fixes #2671
This was before on an iPhone 7 Plus
After
Mobile:
Desktop: