Skip to content
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

Fix Edit Widget's border area around text #702

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ryuukk
Copy link
Contributor

@ryuukk ryuukk commented Sep 24, 2024

Before/After:

image

@ryuukk
Copy link
Contributor Author

ryuukk commented Sep 24, 2024

Unless my assumption about "border" is wrong, is it supposed to be outer or inner? seems like it does both, so perhaps only "half" could shrink the area?

@RobLoach
Copy link
Contributor

Looks like this was from #592 . Anywhere else we need to update here? Thanks for investigating.

@RobLoach
Copy link
Contributor

RobLoach commented Sep 30, 2024

I had a test on this, and it seems to work for me. Anything else we should consider before merging?

As for which value is for which, I'd love for us to move to a similar to the css...

Screenshot from 2024-09-30 13-01-50

@RobLoach RobLoach changed the title border shouldn't affect edit's area Fix Edit Widget's border area around text Dec 7, 2024
@RobLoach
Copy link
Contributor

RobLoach commented Dec 7, 2024

While this looks okay to me, I think it would be good if we got some others to take a look. Thanks!

@riri
Copy link
Contributor

riri commented Dec 8, 2024

Not sure what is expected in fact, as @ryuukk observed: border is inner, outer, or half the way? I haven't dig in the code parts to check.

I pretty agree on a clear definition of bound parts like in CSS, but even there all depends on the box-sizing property, and that's not an easy choice.

In one hand, a border-box style would be easier to figure out a layout because borders wouldn't interfere with the outer view of content size, but on another hand with content-box equivalent, available content area is minus the border size, which is harder to determine which room is given.

Choosing in between would make it worse (in my opinion) and nondeterministic because one should always compute content area bound rect on half the border size.

Maybe we need to think deeper about it and make tries and guesses.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants