Closed
Bug 592562
Opened 14 years ago
Closed 14 years ago
RTL small UI issues after the landing of bug 476423
Categories
(Firefox for Android Graveyard :: General, defect)
Firefox for Android Graveyard
General
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: vingtetun, Assigned: vingtetun)
Details
Attachments
(1 file, 1 obsolete file)
12.76 KB,
patch
|
mfinkle
:
review+
|
Details | Diff | Splinter Review |
There is still some UI problems when launching Fennec in RTL mode. The patch fix many little CSS/UI of them.
Attachment #470998 -
Flags: review?(mark.finkle)
Comment 1•14 years ago
|
||
Comment on attachment 470998 [details] [diff] [review] Patch >- let [,,,controlsW] = Browser.computeSidebarVisibility(); >- this.box.left = window.innerWidth - (this.box.getBoundingClientRect().width + controlsW + margin); >- this.box.top = BrowserUI.starButton.getBoundingClientRect().top + margin; >+ let [,,leftW,rightW] = Browser.computeSidebarVisibility(); >+ let starRect = BrowserUI.starButton.getBoundingClientRect(); >+ let spacer = starRect.left < Elements.tabs.getBoundingClientRect().left ? (leftW + margin) >+ : -(rightW + margin + this.box.getBoundingClientRect().width); >+ this.box.left = starRect.left + spacer; >+ this.box.top = starRect.top + margin; I assume this is because we open the popup "near" the startButton? >diff -r bb790bfcba55 chrome/content/browser.js >- let leftbarCBR = document.getElementById('tabs-container').getBoundingClientRect(); >- let ritebarCBR = document.getElementById('browser-controls').getBoundingClientRect(); >+ let leftbarCBR = Elements.tabs.getBoundingClientRect(); >+ let ritebarCBR = Elements.controls.getBoundingClientRect(); rite* -> right* PLEASE! Don't keep the mistake alive! >diff -r bb790bfcba55 themes/core/browser.css > box[type="documenttab"] { > /* display:block allow us to change the line-height, it won't work otherwise */ > display: block; > width: 128px; >+ margin-left: 8px; > line-height: 0; > } >+box[type="documenttab"]:-moz-locale-dir(rtl) { >+ margin-left: 0px; >+ margin-right: 8px; >+} >+.documenttab-close-container:-moz-locale-dir(rtl) { >+ margin-left: 65px; >+} Can't we use -moz-margin-start or -moz-margin-end here? It seems wrong to be introducing "left" and "right" back into the code. What am I missing?
Assignee | ||
Comment 2•14 years ago
|
||
Addressed comments. (In reply to comment #1) > Comment on attachment 470998 [details] [diff] [review] > Patch > > > >- let [,,,controlsW] = Browser.computeSidebarVisibility(); > >- this.box.left = window.innerWidth - (this.box.getBoundingClientRect().width + controlsW + margin); > >- this.box.top = BrowserUI.starButton.getBoundingClientRect().top + margin; > >+ let [,,leftW,rightW] = Browser.computeSidebarVisibility(); > >+ let starRect = BrowserUI.starButton.getBoundingClientRect(); > >+ let spacer = starRect.left < Elements.tabs.getBoundingClientRect().left ? (leftW + margin) > >+ : -(rightW + margin + this.box.getBoundingClientRect().width); > >+ this.box.left = starRect.left + spacer; > >+ this.box.top = starRect.top + margin; > > I assume this is because we open the popup "near" the startButton? Yep, the (new) patch also fixes some others popups for RTL (NewTabPopup, AlertsContainer)
Attachment #470998 -
Attachment is obsolete: true
Attachment #471066 -
Flags: review?(mark.finkle)
Attachment #470998 -
Flags: review?(mark.finkle)
Comment 3•14 years ago
|
||
Comment on attachment 471066 [details] [diff] [review] Patch v0.2 Can we add a getter for the bookmark popup "box" so the calculation for RTL is only don once? Like you did for the other popups? r+, but if you can tweak the getter, that would be nice. if not, or it's more work, we can file a followup bug.
Attachment #471066 -
Flags: review?(mark.finkle) → review+
Assignee | ||
Comment 4•14 years ago
|
||
(In reply to comment #3) > Comment on attachment 471066 [details] [diff] [review] > Patch v0.2 > > Can we add a getter for the bookmark popup "box" so the calculation for RTL is > only don once? Like you did for the other popups? > > r+, but if you can tweak the getter, that would be nice. if not, or it's more > work, we can file a followup bug. I've tweak the code to go into the getter http://hg.mozilla.org/mobile-browser/rev/13b4414619c2
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•