Skip to content
This repository has been archived by the owner on Jan 17, 2023. It is now read-only.

Fix #3241 : Moved buttons from right to left #3412

Merged
merged 4 commits into from
Aug 29, 2017
Merged

Fix #3241 : Moved buttons from right to left #3412

merged 4 commits into from
Aug 29, 2017

Conversation

ankita240796
Copy link
Contributor

@ankita240796 ankita240796 commented Aug 27, 2017 •
edited
Loading

Changes:-

  1. Shifted all three buttons (cancel,download,save) from right to left
  2. Swapped the position of text and image on save button

@codecov-io
Copy link

codecov-io commented Aug 27, 2017 •
edited
Loading

Codecov Report

Merging #3412 into master will decrease coverage by 0.07%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3412      +/-   ##
==========================================
- Coverage   52.35%   52.28%   -0.08%     
==========================================
  Files          25       25              
  Lines        1425     1425              
  Branches      260      260              
==========================================
- Hits          746      745       -1     
- Misses        679      680       +1
Impacted Files Coverage Δ
server/src/db.js 83.78% <0%> (-0.91%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e21d7b7...d23a761. Read the comment docs.

ianb
ianb previously requested changes Aug 28, 2017
View reviewed changes
Copy link
Contributor

@ianb ianb 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 the PR @ankita240796 ! For #3241 we only want to switch the directions for RTL languages (e.g., Hebrew, Arabic). I believe the iframes have <html dir="rtl">, which you can use in the selector. So the changes you made should only be applied in those cases (like html[dir="rtl"] .highlight-button-save {...})

Note that to test other locales you actually have to download another version of Firefox Nightly, you can get them here – and then use ./bin/run-addon -b PATH_TO_LOCALIZED_FIREFOX

@ankita240796
Copy link
Contributor Author

Requested changes applied

@ianb
Copy link
Contributor

ianb commented Aug 29, 2017

Thanks! The approach looks good, but the button doesn't appear to render quite right. I used an Arabic build, and got this:

image

Note the cloud icon is slightly cut off on the right.

@ankita240796
Copy link
Contributor Author

Made the fix

@ianb ianb dismissed their stale review August 29, 2017 18:10

comments addressed

@ianb ianb merged commit 115d6ed into mozilla-services:master Aug 29, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ianb ianb ianb left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ankita240796 @codecov-io @ianb