textInput pass 3 - adding some styling for background, fix cursor \n issue, and some more cleanup #578

hanbollar · 2024-04-10T17:54:51Z

Linking

related to #513

pass1 was #524, pass2 was #546

all the rest of this section of the pr description is the same as in the #524 / #546 prs

fix selection index is always zero
consolidate input handling if possible (up/down cursor special case)
handle new line wonkiness
background color niceness for visual

Required to Merge

PASS - all necessary actions must pass (excluding the auto-skipped ones)
TEST IN HEADSET - main dev-testing-example and any of the other examples still work as expected
VIDEO - if this pr changes something visually - post a video here of it in headset-MR and/or on desktop (depending on what it affects) for the reviewer to reference.
- textInput pass 3 - adding some styling for background, fix cursor \n issue, and some more cleanup #578 (comment)
TITLE - make sure the pr's title is updated appropriately as it will be used to name the commit on merge
BREAKING CHANGE
- DOCUMENTATION: This includes any changes to html tags and their components
  - make a pr in the documentation repo that updates the manual docs to match the breaking change
  - link the pr of the documentation repo here: #pr
  - that pr must be approved by @lobau
- SAMPLES/INDEX.HTML: This includes any changes (html tags or otherwise) that must be done to our landing page submodule as an effect of this pr's updates
  - make a pr in the mrjs landing page repo that updates the landing page to match the breaking change
  - link the pr of the landing page repo here: #pr
  - that pr must be approved by @hanbollar

Signed-off-by: hanbollar <[email protected]>

render · 2024-04-10T17:54:53Z

Your Render PR Server URL is https://examples-mrjs-pr-578.onrender.com.

Follow its progress at https://dashboard.render.com/static/srv-cobd4r8cmk4c73fnfdsg.

Signed-off-by: hanbollar <[email protected]>

hanbollar · 2024-04-10T22:49:54Z

pausing on this for a short bit

hanbollar · 2024-04-11T01:58:48Z

todos:

make the text size styling look nice for the input
need good default options for the inputs
selection index might be a dangling issue after this pr
- new issue created: selection index for textinput is wrong - not sure why #585

Signed-off-by: hanbollar <[email protected]>

hanbollar · 2024-04-12T22:07:27Z

if all is good - then this is enough to re-expose this as a tag and in documentation repo with future issues created off of this for more work

Signed-off-by: hanbollar <[email protected]>

hanbollar · 2024-04-12T22:10:39Z

if all is good - then this is enough to re-expose this as a tag and in documentation repo with future issues created off of this for more work

future niceties include:

text change if a password input
red outline as something people can toggle (troika has an easy outline feature)
css change for if the field is disabled
selection index for textinput is wrong - not sure why #585
...

i'll make individual issues off of these once this pr is merged for easier tracking

Signed-off-by: hanbollar <[email protected]>

michaelthatsit · 2024-04-12T23:50:00Z

Screen.Recording.2024-04-12.at.4.48.06.PM.mov

michaelthatsit · 2024-04-12T23:50:54Z

Still looking at the code but the cursor seems a little higher than it should be. Also did you test with other font sizes?

hanbollar · 2024-04-12T23:56:19Z

Still looking at the code but the cursor seems a little higher than it should be. Also did you test with other font sizes?

the cursor looks higher because im sort of faking the background height (the coloring one) to fit it decently with like a slight offset for niceties, but that nicety can probably be improved compared to the quick way im doing it (it's the background scale offset in textsystem)- the cursor currently matches the specific text location, with an x offset of cursor width so there isnt touching at least

redo: i had misinterpretated your cursor statement --> actual response:

Yea the cursor issue is a little off because i had issue with line height directly (ie grabbing the difference about the actual troika selection object top and top of the troika represented line itself) - if you have an idea for how to do that im all ears, the conversion was a bit wonky

and yea had tested with other font sizes - it doesnt scale exactly as i want atm, but it does at least start at the right top and width position - was leaving that as a future noodle to not be a blocker on this one

michaelthatsit · 2024-04-13T00:07:02Z

src/core/entities/MRTextAreaEntity.js

-        document.body.appendChild(inputElement); // Ensure it's part of the DOM for event capturing
+
+        // Ensure it's part of the DOM for event capturing
+        document.body.appendChild(inputElement);


I think we talked about this but why aren't we putting this in the shadow root? might scale a little better. especially for embedded experiences.

oh no main reason - i can swap it to that

Yeah it shouldn't be a huge issue and then you wouldn't need to do the 9999999/-99999999 absolute positions. you can just set it to display:none in CSS.

that is so much cleaner haha

Yeah use this as an example #578 (comment)

It's one of those stack overflow tricks I picked up the first time I was trying with MRTextField.

michaelthatsit · 2024-04-13T00:09:04Z

src/core/entities/MRTextFieldEntity.js

+        this.lineHeight = 1.2; // Default line height, can be adjusted as needed
+        this.object3D.name = 'textField';
+
+        // this.wrapper = this.shadowRoot.appendChild(document.createElement('div'));


Related to my other comment. why a hidden field in the DOM instead of an actual field in the shadow?

michaelthatsit · 2024-04-13T00:09:45Z

src/core/entities/MRTextFieldEntity.js

@@ -73,7 +107,7 @@ export class MRTextFieldEntity extends MRTextInputEntity {

        // Ensure the cursor position is updated to reflect the current caret position
        setTimeout(() => {


what's this for?

if we end up needing to pass the event info/etc to the function within that, we want the timeout to have a delay s.t. the event actually holds all the proper information (since it's a press/trigger type one) otherwise, the events can get delayed so the next time you grab the info, you're grabbing the previous version until the next event gets called (it's like the stack gets delayed by one or more steps if that makes sense)

tldr atm this setup is no longer needed since i dont pass the 'event' object to the cursor function anymore, but if we ever do it again, it will be needed with something > 0

^ given this, lmk if you'd want it removed or kept just in case

I'd say if we're not using it we can lose it for the sake of cleanliness

michaelthatsit · 2024-04-13T00:22:12Z

Overall it's pretty solid. I don't think you're blocking anything ATM so try and make the Shadow DOM changes we discussed.

Some notes for what I think we need by v1:

select cursor position (blocked by click events I think)
Text selection
copy/paste
bonus: spell check

Signed-off-by: hanbollar <[email protected]>

michaelthatsit · 2024-04-13T00:32:39Z

Accidental caps but just noticed that text doesn't stay in the textarea if you type too far.

hanbollar · 2024-04-13T00:36:06Z

Accidental caps but just noticed that text doesn't stay in the textarea if you type too far.

yupyup - one of the few task for after this was to add in a scrolling offset - so that the number of lines are only limited to what can show up in the main area but the text can be handled properly (ie textobj only shows what's visible but the hiddenInput can handle the whole amount), so i didnt make a change to limit the height of it based on text (we also cant do mask of a mask either, so that wouldnt hv been worth it)

if that's a blocker for this pr lmk, but i had added in the todos within the code as a task after this pr

michaelthatsit · 2024-04-13T00:39:42Z

No blocker but let's prioritize it on the next pass. Good work!

start

919cd66

Signed-off-by: hanbollar <[email protected]>

hanbollar marked this pull request as draft April 10, 2024 17:54

hanbollar added 14 commits April 10, 2024 11:02

move the one use set text color out of utils

2bed989

Signed-off-by: hanbollar <[email protected]>

fix

c9e57e1

Signed-off-by: hanbollar <[email protected]>

temp save

a6d417a

Signed-off-by: hanbollar <[email protected]>

Merge branch 'main' into hb-input-pass3

33142b1

save

88cc49c

Signed-off-by: hanbollar <[email protected]>

fix the focus/blur not existing call

00b784b

Signed-off-by: hanbollar <[email protected]>

temp save

7e7e915

Signed-off-by: hanbollar <[email protected]>

fixed cursor movement around \n

2a86af0

Signed-off-by: hanbollar <[email protected]>

cleanup && default backcolor

9f7e060

Signed-off-by: hanbollar <[email protected]>

start cleanup

16fb093

Signed-off-by: hanbollar <[email protected]>

everything filled out

fb10049

Signed-off-by: hanbollar <[email protected]>

prettier-fix

5a6d27e

Signed-off-by: hanbollar <[email protected]>

temp save - oging to need defaults

3317e19

Signed-off-by: hanbollar <[email protected]>

start on the cursor pos

7ee79a3

Signed-off-by: hanbollar <[email protected]>

hanbollar added 7 commits April 12, 2024 13:58

temp save

7010f19

Signed-off-by: hanbollar <[email protected]>

Merge branch 'main' into hb-input-pass3

da70a56

update build

dc524b4

Signed-off-by: hanbollar <[email protected]>

background style

20cd7ff

Signed-off-by: hanbollar <[email protected]>

update

3169b39

Signed-off-by: hanbollar <[email protected]>

update the text.html documentation description

2c4a3ac

Signed-off-by: hanbollar <[email protected]>

more writing

75fbf97

Signed-off-by: hanbollar <[email protected]>

hanbollar mentioned this pull request Apr 12, 2024

selection index for textinput is wrong - not sure why #585

Open

1 task

hanbollar changed the title ~~WIP - input pass 3~~ textInput pass 3 - adding some styling for background, fix cursor \n issue, and some more cleanup Apr 12, 2024

prettier-fix

bebf69b

Signed-off-by: hanbollar <[email protected]>

lint-fix

01056fc

Signed-off-by: hanbollar <[email protected]>

remove the debug flag

eaddbd3

Signed-off-by: hanbollar <[email protected]>

hanbollar marked this pull request as ready for review April 12, 2024 22:14

hanbollar requested a review from michaelthatsit April 12, 2024 22:14

michaelthatsit reviewed Apr 13, 2024

View reviewed changes

hanbollar added 4 commits April 12, 2024 17:24

found some words to swap

d30e6e1

Signed-off-by: hanbollar <[email protected]>

address comments

9abd6ef

Signed-off-by: hanbollar <[email protected]>

it's happy so remove old doc.body.appendChild line

ca9aec0

Signed-off-by: hanbollar <[email protected]>

fix for <></> to show up in the text

88169e5

Signed-off-by: hanbollar <[email protected]>

hanbollar merged commit f4e896d into main Apr 13, 2024
6 checks passed

hanbollar deleted the hb-input-pass3 branch April 13, 2024 00:42

hanbollar mentioned this pull request Apr 13, 2024

mr-textarea and mr-textfield are broken #513

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

textInput pass 3 - adding some styling for background, fix cursor \n issue, and some more cleanup #578

textInput pass 3 - adding some styling for background, fix cursor \n issue, and some more cleanup #578

hanbollar commented Apr 10, 2024 •

edited

Loading

render bot commented Apr 10, 2024

hanbollar commented Apr 10, 2024

hanbollar commented Apr 11, 2024 •

edited

Loading

hanbollar commented Apr 12, 2024

hanbollar commented Apr 12, 2024 •

edited

Loading

michaelthatsit commented Apr 12, 2024

michaelthatsit commented Apr 12, 2024

hanbollar commented Apr 12, 2024 •

edited

Loading

michaelthatsit Apr 13, 2024

hanbollar Apr 13, 2024

michaelthatsit Apr 13, 2024

hanbollar Apr 13, 2024

michaelthatsit Apr 13, 2024

michaelthatsit Apr 13, 2024

michaelthatsit Apr 13, 2024

hanbollar Apr 13, 2024

michaelthatsit Apr 13, 2024

michaelthatsit commented Apr 13, 2024

michaelthatsit commented Apr 13, 2024

hanbollar commented Apr 13, 2024 •

edited

Loading

michaelthatsit commented Apr 13, 2024

		@@ -73,7 +107,7 @@ export class MRTextFieldEntity extends MRTextInputEntity {

		// Ensure the cursor position is updated to reflect the current caret position
		setTimeout(() => {

textInput pass 3 - adding some styling for background, fix cursor \n issue, and some more cleanup #578

textInput pass 3 - adding some styling for background, fix cursor \n issue, and some more cleanup #578

Conversation

hanbollar commented Apr 10, 2024 • edited Loading

Linking

Required to Merge

render bot commented Apr 10, 2024

hanbollar commented Apr 10, 2024

hanbollar commented Apr 11, 2024 • edited Loading

hanbollar commented Apr 12, 2024

hanbollar commented Apr 12, 2024 • edited Loading

michaelthatsit commented Apr 12, 2024

michaelthatsit commented Apr 12, 2024

hanbollar commented Apr 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelthatsit commented Apr 13, 2024

michaelthatsit commented Apr 13, 2024

hanbollar commented Apr 13, 2024 • edited Loading

michaelthatsit commented Apr 13, 2024

hanbollar commented Apr 10, 2024 •

edited

Loading

hanbollar commented Apr 11, 2024 •

edited

Loading

hanbollar commented Apr 12, 2024 •

edited

Loading

hanbollar commented Apr 12, 2024 •

edited

Loading

hanbollar commented Apr 13, 2024 •

edited

Loading