Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(4)

Issue 113583: Draw the big arrow animation on the right side on RTL locales. (Closed)

Created:
11 years, 7 months ago by Yusuke (unused)
Modified:
9 years, 7 months ago
Reviewers:
xji, idana
CC:
chromium-reviews_googlegroups.com, jeremy
Visibility:
Public.

Description

Draw the big arrow animation on the right side on RTL locales. > 1) The item drop down arrow resides on the right hand side of the item and not the left hand side. > 2) The animation is drawn on the left and side instead of the right hand side. This patch only fixes the issue 2) above. I'll fix 1) in separate change list. BUG=6103

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+3 lines, -1 line) Patch
M chrome/browser/views/download_started_animation_win.cc View 1 chunk +3 lines, -1 line 2 comments Download

Messages

Total messages: 9 (0 generated)
Yusuke (unused)
Hi Idan, Could you review this patch? This is a partial fix for BUG 6103. ...
11 years, 7 months ago (2009-05-19 14:32:05 UTC) #1
jeremy
+xji as a reviewer Yusuke: thanks for fixing this, could you add xji as a ...
11 years, 7 months ago (2009-05-19 16:32:17 UTC) #2
xji
LGTM. This will place the animation on the right for RTL locale. And you will ...
11 years, 7 months ago (2009-05-19 18:34:44 UTC) #3
idana
LGTM You could use View::MirroredLeftPointForRect using the tab_contents_ View to calculate the x position, but ...
11 years, 7 months ago (2009-05-19 20:43:05 UTC) #4
Yusuke (unused)
> Yusuke: thanks for fixing this, could you add xji as a reviewer on RTL ...
11 years, 7 months ago (2009-05-20 00:26:55 UTC) #5
Yusuke (unused)
Yes. I'll modify the DownloadItemView class in another change. Modified the change list description above ...
11 years, 7 months ago (2009-05-20 00:27:44 UTC) #6
Yusuke (unused)
Idan, Jeremy, Xiaomei, Thanks for your review and suggestions. > You could use View::MirroredLeftPointForRect using ...
11 years, 7 months ago (2009-05-20 00:31:02 UTC) #7
Yusuke (unused)
Forgot to mention this: try succeeded on Win, Mac and Linux. Thanks, Yusuke On 2009/05/20 ...
11 years, 7 months ago (2009-05-20 07:21:22 UTC) #8
Yusuke (unused)
11 years, 7 months ago (2009-05-22 05:53:17 UTC) #9
Xiaomei,
Thanks for the submission (r16616). I'll close this review.

--Yusuke

On 2009/05/20 07:21:22, Yusuke wrote:
> Forgot to mention this:
> try succeeded on Win, Mac and Linux.
> 
> Thanks,
> Yusuke
> 
> On 2009/05/20 00:31:02, Yusuke wrote:
> > Idan, Jeremy, Xiaomei,
> > Thanks for your review and suggestions.
> > 
> > > You could use View::MirroredLeftPointForRect using the tab_contents_ View
to
> > > calculate the x position, but doing this will require converting the point
> > first
> > > so that it is in the parent's coordinate system, which sounds like too
much
> > > effort so I am fine with leaving the code as is.
> > 
> > I'd like to keep the code as is this time.
> > 
> > Could you submit the patch when you have time? as I don't have committer
right
> > yet.
> > 
> > Thanks in advance,
> > Yusuke

Powered by Google App Engine
This is Rietveld 408576698