[Clank] Introduce builder pattern for TextBubble [chromium/src : main]

0 views

Skip to first unread message

Piotr Kotynia (Gerrit)

unread,

9:32 AM (6 hours ago) 9:32 AM

to Boris Sazonov, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, ios-r...@chromium.org, srahim...@chromium.org, devtools...@chromium.org, marq+...@chromium.org, ios-revie...@chromium.org, davidj...@chromium.org, dfried...@chromium.org, dtraino...@chromium.org, estali...@chromium.org, gbb-cct...@google.com, gogeral...@chromium.org, hanxi...@chromium.org, mattsimm...@chromium.org, meilian...@chromium.org, peilinwa...@google.com, wychen...@chromium.org, yuezhang...@chromium.org, yusufo...@chromium.org

Attention needed from Boris Sazonov

Piotr Kotynia added 2 comments

File chrome/browser/keyboard_accessory/android/internal/java/src/org/chromium/chrome/browser/keyboard_accessory/bar_component/KeyboardAccessoryIphUtils.java

Line 196, Patchset 3: // TODO(crbug.com/505624919): Replace with Builder pattern.

Boris Sazonov . resolved

Please migrate at least one usage to the Builder pattern to demonstrate its usage.

Piotr Kotynia

Done

File components/browser_ui/widget/android/java/src/org/chromium/components/browser_ui/widget/textbubble/TextBubble.java

Line 678, Patchset 3:

        public Builder(Context context, View rootView, RectProvider anchorRectProvider) {
            mContext = context;
            mRootView = rootView;
            mAnchorRectProvider = anchorRectProvider;
        }

Boris Sazonov . resolved

The main benefit of the old constructors: while unwieldy, they were forcing the caller to provide the string, the a11y string and `isAccessibilityEnabled` boolean. It seems wrong to create a `TextBubble` without any string, so should we consider adding those params here as well?

Piotr Kotynia

Done

Open in Gerrit

Related details

Attention is currently required from:

Boris Sazonov

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Reply all

Reply to author

Forward

0 new messages