-
Notifications
You must be signed in to change notification settings - Fork 26
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
[SFS-1685] Feature/height siteeditor #96
base: master
Are you sure you want to change the base?
Conversation
# Conflicts: # CHANGELOG.md # messages/context.json # messages/en.json
Hi! I'm VTEX IO CI/CD Bot and I'll be helping you to publish your app! 🤖 Please select which version do you want to release:
And then you just need to merge your PR when you are ready! There is no need to create a release commit/tag.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took a slightly different approach to the solution and committed directly to the branch.
To prevent dimensions from being added incorrectly to stores, we'll have some conditionals that need to be met for them to be applied. You can test it here: https://iespinozaimage--koestore.myvtex.com/?v=12313
-
The dimensions must be explicitly added to the store; they cannot be added with percentages (%). If any dimension has a percentage, the code will not apply the dimensions to the HTML.
-
I maintained the default behavior of adding dimensions to CSS inline if they are not explicitly applied.
-
If dimensions are explicitly added, the CSS inline will only define the maximum size of the dimensions.
-
I added a new field called specificHeight, which is a height specifically defined within the image, without initially assigning it to the value of height, as we don't know the implications of doing so. I maintained the original field definition inherited from the parent (list-context).
-
The priority order for height is: specificHeight (image component height), genericHeight (value of height that can be sent if not set by specificHeight; this was the default previously, so if no specificHeight is set, it will remain), and maxHeight (parent height, list-context).
-
Due to the possibility of empty values being allowed, some conditionals for falsy checks were added.
What problem is this solving?
Hi, this PR tries to solve these two:
#87
#90
How to test it?
https://vysk2--hunterdouglasnl.myvtex.com/
https://vysk--puntoscolombia.myvtex.com/
Explaining
imageDimensions
is used inline in the tag<img>
. Therefore it overwrites any css without important. That breaks many stores using dynamic dimensions.I was thinking in remove entirely the inline
style={}
, but since it's a commit from 5yo I think should be better tobe planned/tested. Then, the height is removed from the inline rule and added just when the component have
experimentalSetExplicitDimensions
as true.