#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 sysrqb): Replying to [comment:88 gk]: > 364e234d66ec101995e1fae1254e632fd23eb69b -- not okay > > `currentTop` does not really seem to be needed. Deleted now. > > {{{ > 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!). > I deleted the ratio variables because I found the syntax confusing due to additional type casting and needing more nested parentheses. This was included in the accidental-fixup commit, but I can revert this change if you think the ratio variables are more readable. > `// Add a classback for` I guess you meant "callback"? I sure do. > > {{{ > + // 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. I'll add more details in the comment. > > s/in in/is in/ ack. -- Ticket URL: <https://trac.torproject.org/projects/tor/ticket/28329#comment:91> Tor Bug Tracker & Wiki <https://trac.torproject.org/> The Tor Project: anonymity online
_______________________________________________ tor-bugs mailing list tor-bugs@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs