Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BrowserToolbar.Button adds inconvenient padding around image #772

Closed
mcomella opened this issue Sep 12, 2018 · 6 comments

Comments

Projects
None yet
4 participants
@mcomella
Copy link
Contributor

commented Sep 12, 2018

I found my toolbar butons assets were offset when I changed the toolbar height and it seems the reason for that is that there is padding added to the view:

) : Toolbar.ActionButton(imageResource, contentDescription, visible, background, listener) {
override fun createView(parent: ViewGroup): View {
val view = super.createView(parent)
val padding = view.dp(ACTION_PADDING_DP)
view.setPadding(padding, padding, padding, padding)

I want to be able to declare the assets to be a specific size (with #741?) and center them in the toolbar, rather than having confusing padding added.


fwiw, this padding seems to be set in proportion to the toolbar height (which I changed):

private const val ACTION_PADDING_DP = 16
private const val DEFAULT_TOOLBAR_HEIGHT_DP = 56

@mcomella mcomella changed the title BrowserToolbar.Button adds questionable padding around image BrowserToolbar.Button adds inconvenient padding around image Sep 12, 2018

@st3fan st3fan added the <toolbar> label Sep 13, 2018

@pocmo

This comment has been minimized.

Copy link
Contributor

commented Sep 24, 2018

Yeah, BrowserToolbar.Button adds default padding to match the default configuration of the toolbar. You may want to override Toolbar.ActionButton for your own behavior?

@mcomella

This comment has been minimized.

Copy link
Contributor Author

commented Sep 24, 2018

@pocmo pocmo added this to the 0.29 🏁 milestone Oct 22, 2018

@Amejia481

This comment has been minimized.

Copy link
Contributor

commented Oct 23, 2018

@mcomella Having a boolean addDefaultPadding in the constructor that allows you to add or not the padding is flexible enough for you?

    open class Button(
        imageResource: Int,
        contentDescription: String,
        visible: () -> Boolean = { true },
        @DrawableRes background: Int? = null,
        listener: () -> Unit,
        val addDefaultPadding: Boolean = false
    ) : Toolbar.ActionButton(imageResource, contentDescription, visible, background, listener) {
        override fun createView(parent: ViewGroup): View {
            val view = super.createView(parent)

            if(addDefaultPadding) {
                val padding = view.resources.pxToDp(ACTION_PADDING_DP)
                view.setPadding(padding, padding, padding, padding)
            }
            return view
        }
    }
@pocmo

This comment has been minimized.

Copy link
Contributor

commented Oct 23, 2018

I think a more flexible solution would be to allow setting padding and setting it to ACTION_PADDING_DP by default.

@Amejia481

This comment has been minimized.

Copy link
Contributor

commented Oct 23, 2018

👍

Amejia481 added a commit to Amejia481/android-components that referenced this issue Oct 23, 2018

@pocmo pocmo added the 🔌 Connect label Oct 24, 2018

Amejia481 added a commit to Amejia481/android-components that referenced this issue Oct 24, 2018

Amejia481 added a commit to Amejia481/android-components that referenced this issue Oct 24, 2018

Amejia481 added a commit to Amejia481/android-components that referenced this issue Oct 24, 2018

@mcomella

This comment has been minimized.

Copy link
Contributor Author

commented Oct 24, 2018

Amejia481 added a commit to Amejia481/android-components that referenced this issue Oct 25, 2018

Closes mozilla-mobile#772: Allowing to pass padding to BrowserToolbar…
….Button,

Toolbar.ActionButton,Toolbar.ActionToggleButton,
Toolbar.ActionSpace and Toolbar.ActionImage

Amejia481 added a commit to Amejia481/android-components that referenced this issue Oct 26, 2018

Amejia481 added a commit to Amejia481/android-components that referenced this issue Oct 26, 2018

Closes mozilla-mobile#772: Allowing to pass padding to BrowserToolbar…
….Button,

Toolbar.ActionButton,Toolbar.ActionToggleButton, Toolbar.ActionSpace
and Toolbar.ActionImage

Amejia481 added a commit to Amejia481/android-components that referenced this issue Oct 26, 2018

@pocmo pocmo closed this in c20285f Oct 26, 2018

pocmo added a commit that referenced this issue Oct 26, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.