zawadz88 / MaterialPopupMenu

Shows Material popup menus grouped in sections & more
Apache License 2.0
646 stars 57 forks source link

Suggestions #72

Open MFlisar opened 3 years ago

MFlisar commented 3 years ago

1. Use ViewBinding

I'd like to use ViewBinding instead of View in the viewBoundCallbacks - I would integrate this. What do you think about that? This would remove the need for putting findViewById(...) calls into the callbacks.

As a side note, this callback could be used to easily replace all text/color adjustments but this would totally break backwards compatibility. But it would minimize the library functions...

2. Support more gravities

I'd like to add support for top/left/right/bottom and center here: https://github.com/zawadz88/MaterialPopupMenu/blob/777bb862bfdb279303a85326be64aca7d71e69f6/material-popup-menu/src/main/java/androidx/appcompat/widget/MaterialRecyclerViewPopupWindow.kt#L263-L265

3. Height calculation optimisation

I think following can be optimised for large menus:

https://github.com/zawadz88/MaterialPopupMenu/blob/777bb862bfdb279303a85326be64aca7d71e69f6/material-popup-menu/src/main/java/androidx/appcompat/widget/MaterialRecyclerViewPopupWindow.kt#L296

I think it would be save to stop iterating over the adapter items as soon as the total height has reached screen height, am I right?

zawadz88 commented 3 years ago

Hi @MFlisar, Sorry for not replying earlier.

1. Use ViewBinding

I'm rather reluctant in putting that in the SDK so that the library does not add a lot of transitive dependencies to the project's using it. The "view accessor" libraries also have a tendency to change over time, we also have Kotlin extensions, DataBinding, Compose on its way... I would like to avoid that in the SDK so that we don't need to modify it whenever a new version comes up.

2. Support more gravities

Center makes sense but I'm not sure how left/right or rather start/end affect dropDownVerticalOffset?

3. Height calculation optimisation

Makes sense. How do you plan to achieve that? I think we might want to check maxHeight if it's not larger then the current window (that's having multi-window on N+ and multiple screen devices)?

MFlisar commented 3 years ago

1. Use ViewBinding

I'm rather reluctant in putting that in the SDK so that the library does not add a lot of transitive dependencies to the project's using it. The "view accessor" libraries also have a tendency to change over time, we also have Kotlin extensions, DataBinding, Compose on its way... I would like to avoid that in the SDK so that we don't need to modify it whenever a new version comes up.

How about providing a small helper class instead? Instead of exposing a view expose a simple class like following, then you do not need to use ViewBinding inside your library:

class ViewData(
    val view: View,
    val textView: TextView,
    ....
)

2. Support more gravities

Center makes sense but I'm not sure how left/right or rather start/end affect dropDownVerticalOffset?

Actually in my own popup I've ended with following anchor types:

enum class Anchor {
    TopLeft, BottomLeft, TopRight, BottomRight, Center
}

Left, right alone does not make much sense probably...

3. Height calculation optimisation

Makes sense. How do you plan to achieve that? I think we might want to check maxHeight if it's not larger then the current window (that's having multi-window on N+ and multiple screen devices)?

I thought you can stop if you reached the current screens height - I did not think about multi window or multiple screen devices... Maybe something like stopping when reaching the root views height does the trick as well.