Let ToolbarView anchor on an (eventual) non-view LocationBar. [chromium/src : main]
Maks Orlovich (Gerrit)
Maks Orlovich voted and added 1 comment![]( "Open in Gerrit")
Votes added by Maks Orlovich
| Commit-Queue | +1 |
1 comment
};I am not sure of this approach, but chrome/browser/ui/location_bar/location_bar.h isn't supposed to know about views...
Related details
- Tom Lukaszewicz
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Tom Lukaszewicz (Gerrit)
Tom Lukaszewicz added 1 comment![]( "Open in Gerrit")
I am not sure of this approach, but chrome/browser/ui/location_bar/location_bar.h isn't supposed to know about views...
Spoke with @kere...@chromium.org and I think we want to lift the existing `views::BubbleAnchor` out of a views include and perhaps somewhere in `//ui/base/interaction/`. It still depends on a views::View but perhaps we just forward declare this since the anchor is intended to be usable by non-views code also.
wdyt?
Related details
- Keren Zhu
- Maks Orlovich
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Maks Orlovich (Gerrit)
Maks Orlovich added 1 comment![]( "Open in Gerrit")
Tom LukaszewiczI am not sure of this approach, but chrome/browser/ui/location_bar/location_bar.h isn't supposed to know about views...
Spoke with @kere...@chromium.org and I think we want to lift the existing `views::BubbleAnchor` out of a views include and perhaps somewhere in `//ui/base/interaction/`. It still depends on a views::View but perhaps we just forward declare this since the anchor is intended to be usable by non-views code also.
wdyt?
Since it's a typedef of a variant it's not really forward-declarable as far as I know?
Related details
- Keren Zhu
- Tom Lukaszewicz
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Tom Lukaszewicz (Gerrit)
Tom Lukaszewicz added 1 comment![]( "Open in Gerrit")
Tom LukaszewiczI am not sure of this approach, but chrome/browser/ui/location_bar/location_bar.h isn't supposed to know about views...
Maks OrlovichSpoke with @kere...@chromium.org and I think we want to lift the existing `views::BubbleAnchor` out of a views include and perhaps somewhere in `//ui/base/interaction/`. It still depends on a views::View but perhaps we just forward declare this since the anchor is intended to be usable by non-views code also.
wdyt?
Since it's a typedef of a variant it's not really forward-declarable as far as I know?
I think it might work (at least on clang https://godbolt.org/z/re93jxKdW)
Related details
- Keren Zhu
- Maks Orlovich
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Tom Lukaszewicz (Gerrit)
Tom Lukaszewicz added 1 comment![]( "Open in Gerrit")
Tom LukaszewiczI am not sure of this approach, but chrome/browser/ui/location_bar/location_bar.h isn't supposed to know about views...
Maks OrlovichSpoke with @kere...@chromium.org and I think we want to lift the existing `views::BubbleAnchor` out of a views include and perhaps somewhere in `//ui/base/interaction/`. It still depends on a views::View but perhaps we just forward declare this since the anchor is intended to be usable by non-views code also.
wdyt?
Tom LukaszewiczSince it's a typedef of a variant it's not really forward-declarable as far as I know?
I think it might work (at least on clang https://godbolt.org/z/re93jxKdW)
Unless you meant wrapping it another way
Maks Orlovich (Gerrit)
Maks Orlovich added 1 comment![]( "Open in Gerrit")
Tom LukaszewiczI am not sure of this approach, but chrome/browser/ui/location_bar/location_bar.h isn't supposed to know about views...
Maks OrlovichSpoke with @kere...@chromium.org and I think we want to lift the existing `views::BubbleAnchor` out of a views include and perhaps somewhere in `//ui/base/interaction/`. It still depends on a views::View but perhaps we just forward declare this since the anchor is intended to be usable by non-views code also.
wdyt?
Tom LukaszewiczSince it's a typedef of a variant it's not really forward-declarable as far as I know?
Tom LukaszewiczI think it might work (at least on clang https://godbolt.org/z/re93jxKdW)
Unless you meant wrapping it another way
- Keren Zhu
- Tom Lukaszewicz
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Maks Orlovich (Gerrit)
Maks Orlovich added 1 comment![]( "Open in Gerrit")
Tom LukaszewiczI am not sure of this approach, but chrome/browser/ui/location_bar/location_bar.h isn't supposed to know about views...
Maks OrlovichSpoke with @kere...@chromium.org and I think we want to lift the existing `views::BubbleAnchor` out of a views include and perhaps somewhere in `//ui/base/interaction/`. It still depends on a views::View but perhaps we just forward declare this since the anchor is intended to be usable by non-views code also.
wdyt?
Tom LukaszewiczSince it's a typedef of a variant it's not really forward-declarable as far as I know?
Tom LukaszewiczI think it might work (at least on clang https://godbolt.org/z/re93jxKdW)
Maks OrlovichUnless you meant wrapping it another way
I meant w/o repeating the whole definition.
Oh, sorry, I thought you meant forward-declaring BubbleAnchor rather than views::View.
(A lighter header for BubbleAnchor might be nice at any rate; I think the above was my frustration about not being able to forward declare it speaking --- it feels bad to add includes to files where people really kept dependencies down).
Tom Lukaszewicz (Gerrit)
Tom Lukaszewicz added 1 comment![]( "Open in Gerrit")
Tom LukaszewiczI am not sure of this approach, but chrome/browser/ui/location_bar/location_bar.h isn't supposed to know about views...
Maks OrlovichSpoke with @kere...@chromium.org and I think we want to lift the existing `views::BubbleAnchor` out of a views include and perhaps somewhere in `//ui/base/interaction/`. It still depends on a views::View but perhaps we just forward declare this since the anchor is intended to be usable by non-views code also.
wdyt?
Tom LukaszewiczSince it's a typedef of a variant it's not really forward-declarable as far as I know?
Tom LukaszewiczI think it might work (at least on clang https://godbolt.org/z/re93jxKdW)
Maks OrlovichUnless you meant wrapping it another way
Maks OrlovichI meant w/o repeating the whole definition.
Oh, sorry, I thought you meant forward-declaring BubbleAnchor rather than views::View.
(A lighter header for BubbleAnchor might be nice at any rate; I think the above was my frustration about not being able to forward declare it speaking --- it feels bad to add includes to files where people really kept dependencies down).
Ah I see what you mean. Maybe we can define it in a types header or similar (not as good as a forward include but perhaps light enough). No strong opinions but I'd be leaning more in that direction vs wrapping it in a struct to allow it to be forward declared.
Related details
- Keren Zhu
- Maks Orlovich
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |