search

Codeinwp / neve-fse

The theme you already love, built for Full-Site Editing
15 stars 6 forks source link

Reduce button padding on mobile viewport #55

Closed JohnPixle closed 1 year ago

JohnPixle commented 1 year ago

Currently the buttons in Neve FSE have a generous padding (16px 40px). However, in Mobile this results in some issues with button not staying inline.

Screenshot 2023-07-12 at 12 25 27 PM

I was wondering, if for mobile viewport we can have the buttons set to 16px padding (all sides). Please note that this should also apply for theis-style-outline button

Hopefully we will be able to have the buttons inline on mobile.

Screenshot 2023-07-12 at 12 28 55 PM

@HardeepAsrani Let me know your thoughts, and happy to help with clarifications and more info! Looping-in @mghenciu for awareness.

HardeepAsrani commented 1 year ago

@JohnPixle I don't mind really, it's a purely design decision so I'd leave it to you and how you want to proceed with it. 😄

JohnPixle commented 1 year ago

@HardeepAsrani Thanks! I asked because we need some input/help on how / where these mobile values are defined 🙈

To summarise, for mobile viewport we need the buttons set to 16px padding (all sides). And this should also apply for the is-style-outline buttons.

We are sure we want to make the change, but not sure where to add the CSS. For example, I see that the style for outline button is defined in the theme files (screenshot).

Screenshot 2023-07-18 at 11 27 00 AM

Can you point us where to add the mobile values? Thanks in advance!

HardeepAsrani commented 1 year ago

@JohnPixle It's added here: https://github.com/Codeinwp/neve-fse/blob/f01d8627a32f5b8ddd8d1ef05835d8070448dcde/assets/css/src/common/_generic.scss#L76

You can use SASS to have a @media selector.

JohnPixle commented 1 year ago

You can use SASS to have a @media selector.

@HardeepAsrani Can you explain this in Greek for me please, or provide an example snippet? 😄

HardeepAsrani commented 1 year ago

@JohnPixle My bad. 😄 Here's an example of how Bogdan is doing this in another file: https://github.com/Codeinwp/neve-fse/blob/f01d8627a32f5b8ddd8d1ef05835d8070448dcde/assets/css/src/welcome-notice.scss#L1-L18

This should also help: https://sass-lang.com/documentation/at-rules/css/

mghenciu commented 1 year ago

I've made some changes in this PR, @JohnPixle , and used 16px 20px, which should be fine even for smaller Mobile Devices. Let me know if this looks and works as expected.

@HardeepAsrani , I've tried using the DRY principle in CSS, not sure if I managed to achieve perfection :)

JohnPixle commented 1 year ago

Thanks @mghenciu looking good!

mghenciu commented 1 year ago

I'll move this to testing, since Bogdan and John reviewed the changes.