feat: add getters for used Unit to HasSize

[knoobie]

Improves the addition to HasSize for Unit.

• adds getWidthUnit and getHeightUnit for better discoverability
  • Open Questions:
    • Optional vs. null vs. Unit.PIXELS as default?
    • Adding those APIs for Min/Max as well? This would increase the amount of methods quite a lot with less gain
• improve performance of Unit
  • change streaming to simple for-each
  • change ordering of enum values, most used (% > px > others)
  • Open Questions:
    • Should I replace the streaming API in getSize as well?
• adds missing units vw, vh, vmin and vmax

[taefi]

Hi and thanks for the contribution.

My opinion about some of the questions:

Optional vs. null vs. Unit.PIXELS as default?

• I would avoid Unit.PIXELS as default since IMO it creates more confusion. Between, null and Optional.empty I vote for the current implementation.

Adding those APIs for Min/Max as well? This would increase the amount of methods quite a lot with less gain

• To be honest, the already added APIs feels like double edge sword! It feels right to get Unit of the size from the same API that we can get the size itself. One the other hand, these methods are also seems like on-liner utility methods that we can define a lot of them. Since this is a base interface, we need to be cautious about adding so many methods to it.

[knoobie]

@taefi Thanks for your opinion!

I would avoid Unit.PIXELS...

The idea was based on the default implementation of V8 where PIXELS was the default instead of null. Personally I'm not a huge fan of Optional (until java 9) but I used it because it's often used in flow for optional values.

To be honest, the already added APIs feels like double edge sword...

The whole addition was based on the comment of Ben in #8533 (comment) - meaning having a "official" API inside the interface is easier to find for the developer and probably working ;)

Additionally, the HasSize interface is probably going to be cleaned up in the future once #5854 is done.

[pleku]

Sorry that it has taken a while. Only two clarifications for the javadocs would be required

[pleku]

Sigh, test failure should not be related in any way, restarted the build.

[knoobie]

@pleku I'm not sure about the target/2.5 and target/2.6 label because the original PR for this isn't backported or? (#8533)

[pleku]

@pleku I'm not sure about the target/2.5 and target/2.6 label because the original PR for this isn't backported or? (#8533)

It is now as the manual cherry-pick PRs will be merged before this, then the bot will backport this too.

But @knoobie can you please rebase this on top of the master as it now failed for the silly Chrome version test.

[knoobie]

@pleku thanks! Rebased and waiting for the build process to do it's job :)

Edit: Looks like the build process doesn't wanna do it's job today "Caused by: java.lang.AssertionError: startup time expected <= 15000 but was 60409"

[fluorumlabs]

Unit ch is missing from the unit list. While rarely used, I believe it's more popular than, let's say, centimeters :)

[knoobie]

@fluorumlabs Thanks! Found a reason to trigger a build restart :)