Re: [tor-bugs] #28329 [Applications/Tor Browser]: Design TBA+Orbot configuration UI/UX

["Tor Bug Tracker & Wiki" <blackhole@xxxxxxxxxxxxxx>]

#28329: Design TBA+Orbot configuration UI/UX
-------------------------------------------------+-------------------------
 Reporter:  sysrqb                               |          Owner:  tbb-
                                                 |  team
     Type:  enhancement                          |         Status:
                                                 |  needs_revision
 Priority:  Very High                            |      Milestone:
Component:  Applications/Tor Browser             |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  tbb-mobile, ux-team, TBA-a3,         |  Actual Points:
  tbb-8.5-must-alpha, TorBrowserTeam201904       |
Parent ID:                                       |         Points:
 Reviewer:                                       |        Sponsor:
                                                 |  Sponsor8
-------------------------------------------------+-------------------------

Comment (by gk):

 364e234d66ec101995e1fae1254e632fd23eb69b -- not okay

 `currentTop` does not really seem to be needed.

 {{{
         final int expectedHeight = (int)
 (currentWidth*imgHeight/imgWidth);
         final int expectedWidth = (int)
 (currentHeight*imgWidth/imgHeight);
 }}}
 You already calculated the ratios and saved them in variables, no need to
 calculate them again here (and elsewhere!).

 `// Add a classback for` I guess you meant "callback"?

 {{{
 +        //     Adjust the width when the current width is greater than
 the expected
 +        //     width.
 +        //   - The opposite is likely true when the device in in
 landscape mode with
 +        //     respect to the height and width. Adjust the height when it
 is greater
 +        //     than the expected height.
 +        //
 +        // Subtract 1 from the expected value as a way of accounting for
 rounding
 +        // error.
 +        if (currentWidth < (expectedWidth - 1)) {
 +            newHeight = expectedHeight;
 +        } else if (currentHeight < (expectedHeight - 1)) {
 +            newWidth = expectedWidth;
 +        }
 }}}
 That part is a bit dense. I was wondering for a while where the width gets
 actually adjusted when it is greater than expected as neither of your two
 if-clauses is actually checking that as your comment suggests. So, ideally
 your comment would at least explain why this scenario falls into the
 `currentHeight < (expectedHeight -1)` check.

 s/in in/is in/

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/28329#comment:88>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online

_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs