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

Improve TextField.autoSize, implement TextField.background #3163

Merged
merged 3 commits into from Feb 13, 2021

Conversation

adrian17
Copy link
Collaborator

@adrian17 adrian17 commented Feb 8, 2021

First commit makes TextField better respect properties of autoSize: with "left", it extends to the right from _x; with "right", it extends to the left from _x+_width. This doesn't affect text layout; just where the text field appears in the end.
This is not a full fix of any issue; in fact, this doesn't fix completely any issues, just improve them a bit. In particular, various missing parts I noticed:

  • there are minor margin-related differences from Flash (you can see the last digit "0" being cut off on screenshots below; that's also why tests run in approximate mode)
  • text re-layout should occur lazily (think like console.log(el.offsetLeft) triggers reflow in JS), not on each property set
  • autoSize="right" should overrule textFormat.align and align text to right;
  • In Text shown in SWF is not in the right place and cut off, and text shown when hovering is inaccurate too #3156, it still sometimes behaves weirdly and I don't fully understand why.

As for improvements:

Screenshots:

Copy link

@seanpm2001 seanpm2001 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good. Everything is clean and functional.

@kmeisthax
Copy link
Member

kmeisthax commented Feb 11, 2021

Same here.

I'll point out that you should also make sure the other EditText tests don't have a rendering regression. I structured them to double as both traceable and renderable regression tests, so you should run them on Desktop & compare with Flash Player or ruffle/master as well.

@danielhjacobs
Copy link
Contributor

danielhjacobs commented Feb 11, 2021

As the author of #3156 this looks promising, though obviously it would be better for me if it fully fixed that issue.

Semi-related; this may not be the best place for this comment, but I thought I'd mention this since my issue was mentioned:

I added more SWFs with the same problem to the issue. They are nearly identical files, but maybe they will give a clue as to the behavior that you "don't fully understand". I thought they were redundant, but better to include too much detail than too little.

Edit: Further testing revealed a slight difference in the problems with some of the SWFs versus others. See latest comment on the issue for further details since this is not the place for me to mention them.

@adrian17
Copy link
Collaborator Author

I'll point out that you should also make sure the other EditText tests don't have a rendering regression

I hoped the existing tests would "handle themselves" without having me actually look at them (especially if they pass) :)
But sure, I can take a look at them (I don't think most of them actually use autoSize, so hopefully they shouldn't be affected).

@adrian17
Copy link
Collaborator Author

@craniumcadoo FYI:

Upon moving the mouse to hover over (4), then (5), then back to (4), then ... repeatedly, the text just keeps moving further right, until it is eventually offscreen completely.

From what I see, my patch seems to fix this particular behavior.

@adrian17
Copy link
Collaborator Author

Also none of the existing EditText tests are affected, from visual inspection.

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.

None yet

5 participants