neve icon indicating copy to clipboard operation
neve copied to clipboard

EM and REM units in Neve theme settings in addition to PX

Open vytisbulkevicius opened this issue 3 years ago • 0 comments

Description:

User requested that it would be beneficial to have EM and REM units in the Neve theme for font-sizes, padding, and margins. Currently, we have PX only. In older versions of the theme, we had EM units but it was removed. I also heard about this request (to have EM units) during the call initiative but didn't hear much of this request other than that.

Alternatives:

Continue having PX units only.

Requested here - https://wordpress.org/support/topic/hey-neve-team-please-read-this/

vytisbulkevicius avatar Jun 29 '22 07:06 vytisbulkevicius

Maybe try to use https://wordpress.github.io/gutenberg/?path=/docs/components-experimental-unitcontrol--default

cristian-ungureanu avatar Aug 16 '22 12:08 cristian-ungureanu

@mskapusuz Tested and here are my findings:

  • [x] In Blog/Archive > Layout > List spacing, when i use REM units, the changes are not visible in customizer but in frontend it works fine https://vertis.d.pr/6PMBPs (i see it's happening on every control with REM units)
  • [x] For Featured post > Post height the minimum value is 300 for all units https://vertis.d.pr/Ys1BD4 (for PX is fine, but for the rest should be different). Same for Scroll to top > Icon size https://vertis.d.pr/CBQoY5

Besides these, everything seems to work as expected 🚀

You can check the issues on this instance:

 Admin area URL: https://relative-units.s3-tastewp.com/wp-admin 
 Username: irinel 
 Password: AUSMvDJC3-Q 

irinelenache avatar Sep 12 '22 14:09 irinelenache

hey @irinelenache

Thank you for your test.

  • [ ] In Blog/Archive > Layout > List spacing, when i use REM units, the changes are not visible in the customizer but in frontend, it works fine https://vertis.d.pr/6PMBPs (i see it's happening on every control with REM units)

Unfortunately, I couldn't replicate it. I've created a fresh env on Instawp and installed Neve+Neve Pro PRs also WooCommerce. It has worked at the initial. Also, it worked fine for other tries after the first saving. Could you please try again and can you help me about reproducing the issue?

  • [ ] For Featured post > Post height the minimum value is 300 for all units https://vertis.d.pr/Ys1BD4 (for PX is fine, but for the rest should be different). Same for Scroll to top > Icon size https://vertis.d.pr/CBQoY5

Yes, you're right, nice catch! that is related to an ongoing challenge. I created another issue(https://github.com/Codeinwp/neve/issues/3609) yesterday to handle it and talked with @cristian-ungureanu . We'll handle it after the v3.4 release probably.

Currently, there are some controls that have too high a minimum value for REM/EM units.

  • Layout -> Blog/Archive -> Avatar Size (min: 20)
  • Layout -> Scroll to Top -> Icon Size (min: 10)
  • Layout -> Blog/Archive -> Post Height (min: 300)

I think we can keep these like this, user cannot use these controls with relative units until we can handle the https://github.com/Codeinwp/neve/issues/3609 . They can continue to use PX for these controls. Does it make sense for you @cristian-ungureanu or I can rollback relative unit supports for the 3 control in the above?

mskapusuz avatar Sep 12 '22 23:09 mskapusuz

Yes, you're right, nice catch! that is related to an ongoing challenge. I created another issue(https://github.com/Codeinwp/neve/issues/3609)

From Irinel report he is saying that minimum is not right, not maximum. If the user is forced to use an unsable value, the whole control is unusable, hence the bug is valid and the bug should be adressed.

selul avatar Sep 13 '22 05:09 selul

@mskapusuz Checked again and the mentioned issues are fixed, thank you 🚀

The min value should be updated also for Layout > Single Post > Post meta > Avatar size

After this is fixed, we can move this to Ready to merge 👍

irinelenache avatar Sep 13 '22 12:09 irinelenache

@mskapusuz Checked again and the mentioned issues are fixed, thank you 🚀

The min value should be updated also for Layout > Single Post > Post meta > Avatar size

After this is fixed, we can move this to Ready to merge 👍

@irinelenache Thank you for your findings!

I've fixed the issue you've mentioned.

Apart from that, I've added relative CSS units for Custom Post Types (avatar size of the CPT archive/CPT single customizer)

Ready for the new test round 🚀

mskapusuz avatar Sep 13 '22 14:09 mskapusuz

@mskapusuz Had another round of testing and everything seems to work fine 🚀

irinelenache avatar Sep 14 '22 06:09 irinelenache

@irinelenache

After your tests, @cristian-ungureanu and I made some changes and pulled the latest changes from Neve 3.4 branch, these are in the following as roughly:

  • Neve Pro: Relative unit support for Advanced Search Form controls
  • Neve Pro: Relative unit support for Advanced Search Icon controls
  • Neve Pro: Relative unit support for Social Icons controls
  • Neve Pro: Relative unit support for Sub menus controls
  • Neve Pro: % (percentage) unit support for Border Radius controls of the Sub menus
  • Small code refactor on the PR (compatibility supports for old Neve version for Neve v3.3.7 and older)

Could you test again? Thank you!

mskapusuz avatar Sep 14 '22 09:09 mskapusuz

@mskapusuz Checked now and the mentioned points work as expected 👍

irinelenache avatar Sep 14 '22 10:09 irinelenache