search

Lundalogik / lime-elements

Provides reusable web components for Lime CRM
https://lundalogik.github.io/lime-elements/versions/latest
Other
38 stars 12 forks source link

text editor height #2935

Closed Kiarokh closed 2 months ago

Kiarokh commented 2 months ago

Review:

Browsers tested:

(Check any that applies, it's ok to leave boxes unchecked if testing something didn't seem relevant.)

Windows:

Linux:

macOS:

Mobile:

github-actions[bot] commented 2 months ago

Documentation has been published to https://lundalogik.github.io/lime-elements/versions/PR-2935/

Kiarokh commented 2 months ago

@LucyChyzhova I think I know why the issue appears now. I will have to refactor some parts, and remove the wrapping <fieldset />. That was seemingly the problem. It behaves in an illogical way, which made it so hard for me to understand what the reason is. Additionally, the new change with limel-action-bar made things slightly harder to track, since now I had to refactor more stuff, before investigating what's wrong.

The <fieldset /> also makes it impossible to add "resizability" to the component, which might be a desired thing in some scenarios.

So I decided to drop the last commit that allowed the consumers to add a desired max-height, (even though that was the only way to make this work),

So I decided to leave this for review up to this stage. Then refactor the component, and get rid of the <fieldset /> and make the component fully expand and shrink according to its container instead. The amount of change will big considerable. So it's better to do it in a follow up PR.

Please review this, and I'm aware of this existing issue:

https://github.com/Lundalogik/lime-elements/assets/35954987/16c41c57-037b-4e81-b399-2941d6eb6d5e

this will be fixed in the next PR

Befkadu1 commented 2 months ago

It looks we don't have min width and min height. Is it intended? The other Issue I was about to say was mentioned in these conversations

https://github.com/Lundalogik/lime-elements/assets/22968497/61365fb6-29cc-4e54-ae1a-499327890c95

Kiarokh commented 2 months ago

It looks we don't have min width and min height. Is it intended? The other Issue I was about to say was mentioned in these conversations

Good catch. I'll fix these in the next PR. I've changed this so many times, so I don't wanna go back there. I'm gonna refactor it all. So no point in fixing it here

lime-opensource commented 2 months ago

:tada: This PR is included in version 37.26.3 :tada:

The release is available on:

Your semantic-release bot :package::rocket: