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

Add a way to customize popup padding #44

Merged
merged 1 commit into from
Apr 15, 2019
Merged

Add a way to customize popup padding #44

merged 1 commit into from
Apr 15, 2019

Conversation

Tunous
Copy link
Contributor

@Tunous Tunous commented Apr 14, 2019

Added few attributes that allow users to customize the popup padding via style:

  • mpm_paddingBottom
  • mpm_paddingLeft
  • mpm_paddingRight
  • mpm_paddingTop

Example

Copy link
Owner

@zawadz88 zawadz88 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove the lines mentioned in the comment and this should be ready to merge :)

@@ -7,6 +7,10 @@
<item name="mpm_secondaryTextColor">@color/mpm_black_54_opacity</item>
<item name="mpm_activeIconColor">@color/mpm_black_54_opacity</item>
<item name="mpm_separatorColor">@color/mpm_black_12_opacity</item>
<item name="mpm_paddingBottom">@dimen/mpm_popup_menu_vertical_padding</item>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is the default and padding is now set programmatically in MaterialRecyclerViewPopupWindow then the following lines can be removed from mpm_popup_menu.xml:

    android:paddingTop="@dimen/mpm_popup_menu_vertical_padding"
    android:paddingBottom="@dimen/mpm_popup_menu_vertical_padding"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@@ -316,6 +316,23 @@ class DarkActivity : AppCompatActivity() {
popupMenu.show(this@DarkActivity, view)
}

@OnClick(R.id.customPaddingTextView)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding a sample as well!

Copy link
Contributor Author

@Tunous Tunous Apr 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem, although I would suggest you to try and combine Dark/Light activities and their layouts. Right now adding new items to sample is kind of annoying since you have to add everything 2 times. Base abstract class with methods that ask for correct style where necessary could be enough.

But that's just a small suggestion :)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with you. I mean, that's what I would normally do in a normal project.
However, I've noticed that the more complicated the sample app is the higher the likelihood that people won't follow what's going on (had some weird questions on some other projects on GitHub...). That's why I went for this awful duplication in the first place :/
It is easier, for complete beginners especially, to just look at the sample code and e.g. copy it to your project.

@zawadz88 zawadz88 added this to the 3.3.0 milestone Apr 15, 2019
@Tunous
Copy link
Contributor Author

Tunous commented Apr 15, 2019

Fixed all issues

@zawadz88 zawadz88 merged commit 2e45c86 into zawadz88:master Apr 15, 2019
@Tunous Tunous deleted the feature/popup-padding branch April 15, 2019 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants