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

Added basic cursor to TextInput #150

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

Added basic cursor to TextInput #150

wants to merge 900 commits into from

Conversation

mjhouse
Copy link

@mjhouse mjhouse commented Apr 10, 2019

I removed a lot of the cruft from the previous iteration of the cursor and tried to limit the changes a little bit. The LayoutResult is being moved into the WindowState and exposed to callbacks. This happens at the bottom of update_display_list, right before it would have gone out of scope- keeping it around shouldn't change performance too much.

Issues:

  1. Cursor height isn't currently set. It's always the same height as the input widget, because I would prefer to set it programmatically but allow users to override it in CSS, and that isn't possible right now as far as I know.

  2. Cursor is a little bit off when text overflows due to Overflowing text in input field is displayed higher than non-overflowing. #142.

  3. The cursor doesn't blink. With CSS this would have required animation/keyframes, and doing it programmatically would have been involved and hacky.

  4. The cursor is always one-character delayed from the real position. This is because:

    1. The glyphs are laid out on the screen and LayoutResult is passed to the WindowState.
    2. The callbacks are triggered, and they calculate the position of the cursor.
    3. The callbacks change the text/position.
    4. -> back to i

    So when the text is displayed, the cursor is always positioned as it would have been in the last render. This is an issue that I'm not sure how to fix. My last attempt at this was re-calculating text width in the TextInputState in order to position the cursor for the next render, but it would be better not to re-do the text layout again if we don't have to.

fschutt and others added 30 commits December 27, 2018 11:00
Screen doesn't redraw on :hover, but selection works properly
Fixed calculator example, implemented proper business logic
Implement background-size:contain and background-size:cover
…peat

This was implemented by using LayoutPrimitiveInfo's clip_rect functionality.
If `no-repeat` is specified in CSS, the rect is clipped to the size of the image on both axis.
If `repeat-x` is specified, the rect's height is clipped to the image height.
If `repeat-y` is specified, the rect's width is clipped to the image width.`
Implement background-repeat: no-repeat | repeat | repeat-x | repeat-y
Due to the instability of crates.io, this gives way better
stability and versioning guarantees with the tradeoff that
now, users might compile crates twice. However, that is
generally better than code breaking every few days, so
it's the lesser evil.

Fixes #86 and #80.
Previously, ":nth-child(1)" did not parse correctly
because the tokenizer was broken to not include the "1"
in the actual parser.

This update also adds support (in the tokenizer, not the parser)
for ::selectors, @ rules in CSS and free-standing string
literals (necessary for @ keyframes and @ media).
Necessary for handling focusing callbacks on text fields and
other, menu-related things.
@fschutt
Copy link
Owner

fschutt commented Apr 10, 2019

Thanks for reworking it, this looks a lot better than the previous attempt, good job. I'll take a look later for minor changes, but overall it looks decent. The "one frame delay" problem - yeah, it's a problem and I'm not sure how to solve it (maybe using inline layout later on instead of absolute / relative positioning, so that the position of the cursor is treated like a "span" element), but it's certainly better than nothing.

I wouldn't necessarily put the LayoutResult it inside the WindowState, I'd just reference it as a new field to the CallbackInfo (like struct CallbackInfo { layout: &'a LayoutResult }) - I essentially want the CallbackInfo to be more or less like the "inspect element" tool, so that you can query the computed layout (the position / size of on-screen rectangles) and computed CSS properties. I'd avoid mutating / setting the window state if possible, because (a) the window state should only describe the actual window, i.e the position / size of the actual window and (b) mutating things leads to bugs very quickly, ex. when in a future revision someone accidentally forgets to update the window state it can lead to update bugs.

But for a first attempt it's not too bad.

@mjhouse
Copy link
Author

mjhouse commented Apr 10, 2019

Thanks, I'll go look at the CallbackInfo and find a logical place to add the LayoutResult to it.

Edit: a logical place in the code to snag the LayoutResult, I mean.

@mjhouse
Copy link
Author

mjhouse commented Apr 11, 2019

I moved the LayoutResult to UiState so that it gets passed to callbacks in CallbackInfo. Since the render happens after the callbacks, I had to put it on some existing struct so that it would be available in the next pass.

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.