ripe-tech / ripe-sdk

The public Javascript SDK for RIPE Core
https://www.platforme.com
Apache License 2.0
8 stars 4 forks source link

fix: remove default width and height for configurator #349

Closed BeeMargarida closed 2 years ago

BeeMargarida commented 2 years ago
- -
Issue Since the width and height are set has default, it did not defaulted to the clientWidth (space available) of the configurator element, which caused regressions on ripe-green:
imagem
https://github.com/ripe-tech/ripe-green/pull/113#issuecomment-1014477503
Dependencies --
Decisions - Remove default width and height of the configurator.
Animated GIF Now:
imagem
ripe-tobias-bot[bot] commented 2 years ago

Woof, Woof!

Thank you for submitting the "fix: remove default width and height for configurator" pull request 😎.

Please do not forget to review our internal guidelines:

Engaging in the development process in the best possible way helps it being efficient and fast.

Your friend, Tobias (Platforme's mascot)

Tobias Bot
joamag commented 2 years ago

@BeeMargarida I'm not sure this is the best way to go, overriding this old parameters is tricky

BeeMargarida commented 2 years ago

@joamag The reasoning I had to remove this was that, before supporting width and height in resize and in the sizing of the configurator, width and height were not used in resize and the size defaulted to element.clientWidth if not defined. https://github.com/ripe-tech/ripe-sdk/blob/97e13e22aab4c13612f69813562d0a4881262551/src/js/visual/configurator-prc.js#L380-L387 In the other parts that width and height were defined, they always defaulted to size first https://github.com/ripe-tech/ripe-sdk/blob/97e13e22aab4c13612f69813562d0a4881262551/src/js/visual/configurator-prc.js#L1172-L1174

Since the size in resize is defaulting to clientWidth, taking the available space, perhaps width and height should do the same (if no value is provided). Because right now, there is no way to do that if no size, width and height is passed, it will always default to 1000x1000 instead of the space available.