Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2904173002:
Do peeking animation when target language truncated (Translate infobar) (Closed)
|
Created:
3 years, 6 months ago by Marti Wong Modified:
3 years, 6 months ago CC:
chromium-reviews, agrieve+watch_chromium.org, dfalcantara+watch_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionDo peeking animation when target language tab truncated.
When the translation infobar isn't long enough, the target
language tab might be hidden and user would not know it
exists.
The solution is to do a 'peeking' animation which scrolls
the tabLayout to the end to show its hidden part.
'Peeking animation' consists of the following steps:
1. wait for 1000ms
2. scroll to the end in 300ms
3. wait for 1000ms
4. scroll back to the start in 300ms
If this page is 'always translate', step 3, 4 will be
skipped.
Condition to trigger 'Peeking animation':
1. when >50% of second tab is invisible OR
2. this page is 'always translate'
Recorded mp4:
https://drive.google.com/open?id=0B1O0Z7eoZMuGdjZYSUdmelZiV2M
https://drive.google.com/open?id=0B1O0Z7eoZMuGU2tGWEF1bXFjV0k
BUG=721936
Review-Url: https://codereview.chromium.org/2904173002
Cr-Commit-Position: refs/heads/master@{#477234}
Committed: https://chromium.googlesource.com/chromium/src/+/6be7b0586aabd4abf05c87eaa08f66549f399d33
Patch Set 1 #
Total comments: 6
Patch Set 2 : fix #
Total comments: 14
Patch Set 3 : fix and sync #
Total comments: 8
Patch Set 4 : nit #Patch Set 5 : fix #
Messages
Total messages: 33 (18 generated)
Description was changed from ========== Do peeking animation when target language truncated (Translate infobar) BUG=721936 ========== to ========== When the translation infobar isn't long enough, the target language tab might be truncated and user would not know it exist. The solution is to do a 'peeking' animation when there is truncation of the any tab. 'Peeking animation' consists of the following steps: 1. wait for 500ms 2. scroll to the end in 300ms 3. wait for 1000ms 4. scroll back to the start in 300ms Recorded mp4: https://drive.google.com/open?id=0B1O0Z7eoZMuGdjZYSUdmelZiV2M https://drive.google.com/open?id=0B1O0Z7eoZMuGU2tGWEF1bXFjV0k BUG=721936 ==========
martiw@chromium.org changed reviewers: + googleo@chromium.org
Hi Leo, PTAL thanks https://codereview.chromium.org/2904173002/diff/1/chrome/android/java/src/org... File chrome/android/java/src/org/chromium/chrome/browser/infobar/TranslateCompactInfoBar.java (right): https://codereview.chromium.org/2904173002/diff/1/chrome/android/java/src/org... chrome/android/java/src/org/chromium/chrome/browser/infobar/TranslateCompactInfoBar.java:195: if (left != oldLeft || top != oldTop || right != oldRight || bottom != oldBottom) { This is to check if layout is really changed. This checking is needed because when there is more than one infobar (in baidu.com for example, there will be tranlation infobar and device location infobar). onLayoutChange will be triggered twice (with the exact same left/top/right/bottom) and it will prevent the peeking animation from start playing. This checking is to ensure to do something ONLY when the layout is really changed. https://codereview.chromium.org/2904173002/diff/1/chrome/android/java/src/org... chrome/android/java/src/org/chromium/chrome/browser/infobar/TranslateCompactInfoBar.java:207: mTabLayout.post(new Runnable() { startPeekingAnimationIfNeeded() will measure UI width and it is too early to do that here (UI is not laid out on screen yet). That's why I Use post(new Runnable()...) to ensure startPeekingAnimationIfNeeded() is called after measure/layout phrase. Please advice if this approach is fine. There is another approach OnGlobalLayoutListener which will do the same thing. But it seems more complicated. https://codereview.chromium.org/2904173002/diff/1/chrome/android/java/src/org... chrome/android/java/src/org/chromium/chrome/browser/infobar/TranslateCompactInfoBar.java:219: mTabLayout.stopPeekingAnimationIfPlaying(); Three cases to stop the peeking animation: 1. menu button is clicked (here) 2. close button is clicked (line 314 of this file) 3. any tab is clicked (onInterceptTouchEvent of TranslateTabLayout.java) https://codereview.chromium.org/2904173002/diff/1/chrome/android/java/src/org... File chrome/android/java/src/org/chromium/chrome/browser/infobar/translate/TranslateTabLayout.java (right): https://codereview.chromium.org/2904173002/diff/1/chrome/android/java/src/org... chrome/android/java/src/org/chromium/chrome/browser/infobar/translate/TranslateTabLayout.java:53: To measure the maximum scrolling width, I need to measure the width of all tabs in the tabStrip. Unfortunately, mTabStrip of TabLayout is private and there is no way to measure tab widths directly. That's why I need to store the tabPaddings here and later use it to measure tab widths by adding PaddingStart + ContentWidth + PaddingEnd. https://codereview.chromium.org/2904173002/diff/1/chrome/android/java/src/org... chrome/android/java/src/org/chromium/chrome/browser/infobar/translate/TranslateTabLayout.java:201: ObjectAnimator.ofInt(this, "scrollX", isRtl ? 0 : maxScrollDistance); set scrollX = 0 will scroll the tabLayout to the left no matter it is RTL or LTR.
On 2017/05/26 05:16:30, Marti Wong wrote: > Hi Leo, PTAL > thanks > > https://codereview.chromium.org/2904173002/diff/1/chrome/android/java/src/org... > File > chrome/android/java/src/org/chromium/chrome/browser/infobar/TranslateCompactInfoBar.java > (right): > > https://codereview.chromium.org/2904173002/diff/1/chrome/android/java/src/org... > chrome/android/java/src/org/chromium/chrome/browser/infobar/TranslateCompactInfoBar.java:195: > if (left != oldLeft || top != oldTop || right != oldRight || bottom != > oldBottom) { > This is to check if layout is really changed. > This checking is needed because when there is more than one infobar (in > http://baidu.com for example, there will be tranlation infobar and device location > infobar). > onLayoutChange will be triggered twice (with the exact same > left/top/right/bottom) and it will prevent the peeking animation from start > playing. > This checking is to ensure to do something ONLY when the layout is really > changed. > > https://codereview.chromium.org/2904173002/diff/1/chrome/android/java/src/org... > chrome/android/java/src/org/chromium/chrome/browser/infobar/TranslateCompactInfoBar.java:207: > mTabLayout.post(new Runnable() { > startPeekingAnimationIfNeeded() will measure UI width and it is too early to do > that here (UI is not laid out on screen yet). > That's why I Use post(new Runnable()...) to ensure > startPeekingAnimationIfNeeded() is called after measure/layout phrase. > > Please advice if this approach is fine. > > There is another approach OnGlobalLayoutListener which will do the same thing. > But it seems more complicated. > > https://codereview.chromium.org/2904173002/diff/1/chrome/android/java/src/org... > chrome/android/java/src/org/chromium/chrome/browser/infobar/TranslateCompactInfoBar.java:219: > mTabLayout.stopPeekingAnimationIfPlaying(); > Three cases to stop the peeking animation: > 1. menu button is clicked (here) > 2. close button is clicked (line 314 of this file) > 3. any tab is clicked (onInterceptTouchEvent of TranslateTabLayout.java) > > https://codereview.chromium.org/2904173002/diff/1/chrome/android/java/src/org... > File > chrome/android/java/src/org/chromium/chrome/browser/infobar/translate/TranslateTabLayout.java > (right): > > https://codereview.chromium.org/2904173002/diff/1/chrome/android/java/src/org... > chrome/android/java/src/org/chromium/chrome/browser/infobar/translate/TranslateTabLayout.java:53: > > To measure the maximum scrolling width, I need to measure the width of all tabs > in the tabStrip. > > Unfortunately, mTabStrip of TabLayout is private and there is no way to measure > tab widths directly. > > That's why I need to store the tabPaddings here and later use it to measure tab > widths by adding PaddingStart + ContentWidth + PaddingEnd. > > https://codereview.chromium.org/2904173002/diff/1/chrome/android/java/src/org... > chrome/android/java/src/org/chromium/chrome/browser/infobar/translate/TranslateTabLayout.java:201: > ObjectAnimator.ofInt(this, "scrollX", isRtl ? 0 : maxScrollDistance); > set scrollX = 0 will scroll the tabLayout to the left no matter it is RTL or > LTR. Thanks for the experiment. I believe you could save lots of code by something below: mView.animate().translationY(100).setDuration(3000).setStartDelay(2000).start(); API docs https://developer.android.com/reference/android/view/ViewPropertyAnimator.html And there are 3 things to take care. 1, Whether the animation start moment is correct or not. 2, What kinds of condition will trigger the animation (Eg. first tab took all the tablayout's space or 95% of the whole space) 3, In the auto translate case, you should stop at the end. Hope it's helpful
> I believe you could save lots of code by something below: > mView.animate().translationY(100).setDuration(3000).setStartDelay(2000).start(); > API docs > https://developer.android.com/reference/android/view/ViewPropertyAnimator.html Thanks for the tip!! Just do some experiments and it's unlucky that View.animate() doesn't have something like TranslationScrollX therefore I could not use it here. Otherwise, it could save lots of lines. > And there are 3 things to take care. > 1, Whether the animation start moment is correct or not. Thx. I will do more test to ensure that. > 2, What kinds of condition will trigger the animation (Eg. first tab took all > the tablayout's space or 95% of the whole space) Will discuss with the team to determine the trigger condition. > 3, In the auto translate case, you should stop at the end. So if it is auto translate, the peeking animation will play the "scroll to end" part only? When there are 3 tabs in future, we might still want the 2nd part of the animation but it just"scroll to the target Tab" (instead of scroll to start). We could add a TODO comment.
Discussing with the team about trigger condition. Will update this CL after getting the feedback.
Description was changed from ========== When the translation infobar isn't long enough, the target language tab might be truncated and user would not know it exist. The solution is to do a 'peeking' animation when there is truncation of the any tab. 'Peeking animation' consists of the following steps: 1. wait for 500ms 2. scroll to the end in 300ms 3. wait for 1000ms 4. scroll back to the start in 300ms Recorded mp4: https://drive.google.com/open?id=0B1O0Z7eoZMuGdjZYSUdmelZiV2M https://drive.google.com/open?id=0B1O0Z7eoZMuGU2tGWEF1bXFjV0k BUG=721936 ========== to ========== When the translation infobar isn't long enough, the target language tab might be hidden and user would not know it exist. The solution is to do a 'peeking' animation which scrolls the tabLayout to the end to show its hidden part. 'Peeking animation' consists of the following steps: 1. wait for 1000ms 2. scroll to the end in 300ms 3. wait for 1000ms 4. scroll back to the start in 300ms If this page is 'always translate', step 3, 4 will be skipped. Condition to trigger 'Peeking animation': 1. when >50% of second tab is invisible OR 2. this page is 'always translate' Recorded mp4: https://drive.google.com/open?id=0B1O0Z7eoZMuGdjZYSUdmelZiV2M https://drive.google.com/open?id=0B1O0Z7eoZMuGU2tGWEF1bXFjV0k BUG=721936 ==========
martiw@chromium.org changed reviewers: + dfalcantara@chromium.org
Hi Dan, PTAL. There are some explanation comments on Patchset 1 which might be useful when you are reviewing. Thx. https://codereview.chromium.org/2904173002/diff/1/chrome/android/java/src/org... File chrome/android/java/src/org/chromium/chrome/browser/infobar/TranslateCompactInfoBar.java (right): https://codereview.chromium.org/2904173002/diff/1/chrome/android/java/src/org... chrome/android/java/src/org/chromium/chrome/browser/infobar/TranslateCompactInfoBar.java:207: mTabLayout.post(new Runnable() { In patch set 2, this is changed to 'OnGlobalLayoutListener' because 'post runnable' will cause a bug (called too early) on Nexus 5 when reloading a webpage. https://codereview.chromium.org/2904173002/diff/20001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/infobar/TranslateCompactInfoBar.java (right): https://codereview.chromium.org/2904173002/diff/20001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/infobar/TranslateCompactInfoBar.java:210: // Start peeking animation (if needed) when UI measurement is ready (OnGlobalLayoutListener Use 'OnGlobalLayoutListener' instead of 'post Runnable'. because 'post runnable' will cause a bug (called too early) on Nexus 5 when reloading a webpage. https://codereview.chromium.org/2904173002/diff/20001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/infobar/translate/TranslateTabLayout.java (right): https://codereview.chromium.org/2904173002/diff/20001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/infobar/translate/TranslateTabLayout.java:33: private static final long START_POSITION_WAIT_DURATION_MS = 1000; change to 1000ms (requested by designer)
The CQ bit was checked by martiw@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Start sending these to Matt; he's an owner now.
dfalcantara@chromium.org changed reviewers: + mdjones@chromium.org
-> Matt.
-> Matt.
Add a single-line summary to the top of your commit message (CL title is probably fine). exist -> exists in description. https://codereview.chromium.org/2904173002/diff/20001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/infobar/TranslateCompactInfoBar.java (right): https://codereview.chromium.org/2904173002/diff/20001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/infobar/TranslateCompactInfoBar.java:214: viewTreeObserver.addOnGlobalLayoutListener(new OnGlobalLayoutListener() { Why use this instead of overriding onLayout in TranslateTabLayout or using the listener above? https://codereview.chromium.org/2904173002/diff/20001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/infobar/translate/TranslateTabLayout.java (right): https://codereview.chromium.org/2904173002/diff/20001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/infobar/translate/TranslateTabLayout.java:37: private static final long END_POSITION_WAIT_DURATION_MS = 1000; nit: space after constant and doc (here and above). https://codereview.chromium.org/2904173002/diff/20001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/infobar/translate/TranslateTabLayout.java:171: private int getTabWidth(int position) { nit: javadoc here and below. https://codereview.chromium.org/2904173002/diff/20001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/infobar/translate/TranslateTabLayout.java:197: && (maxScrollDistance > widthOfSecondTab / 2 || scrollToEndOnly)) { Can this condition be inverted to be an early return instead of nesting? https://codereview.chromium.org/2904173002/diff/20001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/infobar/translate/TranslateTabLayout.java:215: mPeekingAnimatorSet.playSequentially(scrollToEndAnimator); nit: I'd consider creating a list and adding the animations to that. At the end of this function you could call playSequentially(list) and avoid this check. https://codereview.chromium.org/2904173002/diff/20001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/infobar/translate/TranslateTabLayout.java:239: public void stopPeekingAnimationIfPlaying() { nit: endPeekingAnimation
Description was changed from ========== When the translation infobar isn't long enough, the target language tab might be hidden and user would not know it exist. The solution is to do a 'peeking' animation which scrolls the tabLayout to the end to show its hidden part. 'Peeking animation' consists of the following steps: 1. wait for 1000ms 2. scroll to the end in 300ms 3. wait for 1000ms 4. scroll back to the start in 300ms If this page is 'always translate', step 3, 4 will be skipped. Condition to trigger 'Peeking animation': 1. when >50% of second tab is invisible OR 2. this page is 'always translate' Recorded mp4: https://drive.google.com/open?id=0B1O0Z7eoZMuGdjZYSUdmelZiV2M https://drive.google.com/open?id=0B1O0Z7eoZMuGU2tGWEF1bXFjV0k BUG=721936 ========== to ========== Do peeking animation when target language tab truncated. When the translation infobar isn't long enough, the target language tab might be hidden and user would not know it exists. The solution is to do a 'peeking' animation which scrolls the tabLayout to the end to show its hidden part. 'Peeking animation' consists of the following steps: 1. wait for 1000ms 2. scroll to the end in 300ms 3. wait for 1000ms 4. scroll back to the start in 300ms If this page is 'always translate', step 3, 4 will be skipped. Condition to trigger 'Peeking animation': 1. when >50% of second tab is invisible OR 2. this page is 'always translate' Recorded mp4: https://drive.google.com/open?id=0B1O0Z7eoZMuGdjZYSUdmelZiV2M https://drive.google.com/open?id=0B1O0Z7eoZMuGU2tGWEF1bXFjV0k BUG=721936 ==========
Patchset #3 (id:40001) has been deleted
Hi Matthew, PTAL again. Thanks so much~! https://codereview.chromium.org/2904173002/diff/20001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/infobar/TranslateCompactInfoBar.java (right): https://codereview.chromium.org/2904173002/diff/20001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/infobar/TranslateCompactInfoBar.java:214: viewTreeObserver.addOnGlobalLayoutListener(new OnGlobalLayoutListener() { On 2017/06/01 18:29:51, mdjones wrote: > Why use this instead of overriding onLayout in TranslateTabLayout or using the > listener above? Done. Thx for the advice. Moved to startPeekingAnimationIfNeeded to onLayoutChange. Added the flag mLayoutChangeFirstTime. https://codereview.chromium.org/2904173002/diff/20001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/infobar/translate/TranslateTabLayout.java (right): https://codereview.chromium.org/2904173002/diff/20001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/infobar/translate/TranslateTabLayout.java:37: private static final long END_POSITION_WAIT_DURATION_MS = 1000; On 2017/06/01 18:29:51, mdjones wrote: > nit: space after constant and doc (here and above). Done. https://codereview.chromium.org/2904173002/diff/20001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/infobar/translate/TranslateTabLayout.java:171: private int getTabWidth(int position) { On 2017/06/01 18:29:51, mdjones wrote: > nit: javadoc here and below. Done. https://codereview.chromium.org/2904173002/diff/20001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/infobar/translate/TranslateTabLayout.java:197: && (maxScrollDistance > widthOfSecondTab / 2 || scrollToEndOnly)) { On 2017/06/01 18:29:51, mdjones wrote: > Can this condition be inverted to be an early return instead of nesting? Done. https://codereview.chromium.org/2904173002/diff/20001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/infobar/translate/TranslateTabLayout.java:215: mPeekingAnimatorSet.playSequentially(scrollToEndAnimator); On 2017/06/01 18:29:51, mdjones wrote: > nit: I'd consider creating a list and adding the animations to that. At the end > of this function you could call playSequentially(list) and avoid this check. Done. Thx for the comment. The code becomes more readable now. see if this is okay. https://codereview.chromium.org/2904173002/diff/20001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/infobar/translate/TranslateTabLayout.java:239: public void stopPeekingAnimationIfPlaying() { On 2017/06/01 18:29:51, mdjones wrote: > nit: endPeekingAnimation Done.
lgtm % nits https://codereview.chromium.org/2904173002/diff/60001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/infobar/TranslateCompactInfoBar.java (right): https://codereview.chromium.org/2904173002/diff/60001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/infobar/TranslateCompactInfoBar.java:200: if (mLayoutChangeFirstTime) { nit: mIsFirstLayout https://codereview.chromium.org/2904173002/diff/60001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/infobar/translate/TranslateTabLayout.java (right): https://codereview.chromium.org/2904173002/diff/60001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/infobar/translate/TranslateTabLayout.java:49: private int mTabPaddingStart; These say 'padding' but aren't applied to any views; they only seem to be used in width calculation. Can we have doc as to what these are? Also, if you want to use javadoc but your doc doesn't take up a full line: /** Some comment. */ to save vertical space. https://codereview.chromium.org/2904173002/diff/60001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/infobar/translate/TranslateTabLayout.java:176: * Calculate and return the width of a specified tab. nit: Might be worth mentioning the padding since it isn't included in the custom view and Tab doesn't provide a means of getting the width. https://codereview.chromium.org/2904173002/diff/60001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/infobar/translate/TranslateTabLayout.java:229: ArrayList<Animator> animators = new ArrayList<Animator>(); nit: List<Animator> animators = new ArrayList<>();
Thanks Matthew~! Committing https://codereview.chromium.org/2904173002/diff/60001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/infobar/TranslateCompactInfoBar.java (right): https://codereview.chromium.org/2904173002/diff/60001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/infobar/TranslateCompactInfoBar.java:200: if (mLayoutChangeFirstTime) { On 2017/06/05 16:05:10, mdjones wrote: > nit: mIsFirstLayout Done. https://codereview.chromium.org/2904173002/diff/60001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/infobar/translate/TranslateTabLayout.java (right): https://codereview.chromium.org/2904173002/diff/60001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/infobar/translate/TranslateTabLayout.java:49: private int mTabPaddingStart; On 2017/06/05 16:05:10, mdjones wrote: > These say 'padding' but aren't applied to any views; they only seem to be used > in width calculation. Can we have doc as to what these are? > > Also, if you want to use javadoc but your doc doesn't take up a full line: > > /** Some comment. */ > > to save vertical space. Done. https://codereview.chromium.org/2904173002/diff/60001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/infobar/translate/TranslateTabLayout.java:176: * Calculate and return the width of a specified tab. On 2017/06/05 16:05:10, mdjones wrote: > nit: Might be worth mentioning the padding since it isn't included in the custom > view and Tab doesn't provide a means of getting the width. Done. https://codereview.chromium.org/2904173002/diff/60001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/infobar/translate/TranslateTabLayout.java:229: ArrayList<Animator> animators = new ArrayList<Animator>(); On 2017/06/05 16:05:10, mdjones wrote: > nit: List<Animator> animators = new ArrayList<>(); Done.
The CQ bit was checked by martiw@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from mdjones@chromium.org Link to the patchset: https://codereview.chromium.org/2904173002/#ps80001 (title: "nit")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by martiw@chromium.org
The CQ bit was checked by martiw@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from mdjones@chromium.org Link to the patchset: https://codereview.chromium.org/2904173002/#ps100001 (title: "fix")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 100001, "attempt_start_ts": 1496734468435440,
"parent_rev": "e38dbf83761a9f1ee952e4f908040d50b34f0b12", "commit_rev":
"6be7b0586aabd4abf05c87eaa08f66549f399d33"}
Message was sent while issue was closed.
Description was changed from ========== Do peeking animation when target language tab truncated. When the translation infobar isn't long enough, the target language tab might be hidden and user would not know it exists. The solution is to do a 'peeking' animation which scrolls the tabLayout to the end to show its hidden part. 'Peeking animation' consists of the following steps: 1. wait for 1000ms 2. scroll to the end in 300ms 3. wait for 1000ms 4. scroll back to the start in 300ms If this page is 'always translate', step 3, 4 will be skipped. Condition to trigger 'Peeking animation': 1. when >50% of second tab is invisible OR 2. this page is 'always translate' Recorded mp4: https://drive.google.com/open?id=0B1O0Z7eoZMuGdjZYSUdmelZiV2M https://drive.google.com/open?id=0B1O0Z7eoZMuGU2tGWEF1bXFjV0k BUG=721936 ========== to ========== Do peeking animation when target language tab truncated. When the translation infobar isn't long enough, the target language tab might be hidden and user would not know it exists. The solution is to do a 'peeking' animation which scrolls the tabLayout to the end to show its hidden part. 'Peeking animation' consists of the following steps: 1. wait for 1000ms 2. scroll to the end in 300ms 3. wait for 1000ms 4. scroll back to the start in 300ms If this page is 'always translate', step 3, 4 will be skipped. Condition to trigger 'Peeking animation': 1. when >50% of second tab is invisible OR 2. this page is 'always translate' Recorded mp4: https://drive.google.com/open?id=0B1O0Z7eoZMuGdjZYSUdmelZiV2M https://drive.google.com/open?id=0B1O0Z7eoZMuGU2tGWEF1bXFjV0k BUG=721936 Review-Url: https://codereview.chromium.org/2904173002 Cr-Commit-Position: refs/heads/master@{#477234} Committed: https://chromium.googlesource.com/chromium/src/+/6be7b0586aabd4abf05c87eaa08f... ==========
Message was sent while issue was closed.
Committed patchset #5 (id:100001) as https://chromium.googlesource.com/chromium/src/+/6be7b0586aabd4abf05c87eaa08f... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
