[HOLD FB #38679] [$1000] iOS - Chat - "Edit comment" input field scrolls up when tap enter in 1st line of text

If you haven’t already, check out our contributing guidelines for onboarding and email<EMAIL_ADDRESS>to request to join our Slack channel!

Action Performed:
Precondition - User sent a text message in chat (e.g. 1 word or 1 line of text)

Open IOS app
Open chat from the precondition
Long press the message
Tap "Edit comment"
Add 6 lines of text below the 1st line so there is a total of 7 lines
Put the cursor at the end of 1st line
Tap "Enter"
Place cursor at the end of 7th line / on the 8th line
9, Place cursor back at the end of the 1st line
Tap "Enter"

Expected Result:
1st line of text hides above and cursor is focus on the next line of text and visible
Actual Result:
At step 7 the comment field scrolls slightly up and cursor also scrolls out of the view.
At step 10 field focus scrolls back to line 7/8
Workaround:
Unknown
Platforms:

Which of our officially supported platforms is this issue occurring on?

[ ] Android / native
[ ] Android / Chrome
[x] iOS / native
[ ] iOS / Safari
[ ] MacOS / Chrome / Safari
[ ] MacOS / Desktop

Version Number: <IP_ADDRESS>
Reproducible in staging?: yes
Reproducible in production?: yes
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
https://platform.applause.com/services/links/v1/external/197b3277eef42e2b057288d9b61067c2fce3fc4a98ffe01c313143ea19511833
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:
View all open jobs on GitHub
Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~0113571ce6c5a8b04e
Upwork Job ID:<PHONE_NUMBER>930722304
Last Price Increase: 2023-07-28
        2023-07-19Automatic offers:

 |  | 0
fabriziobertoglio1987 | Contributor | 25744429

I was assigned when I was offline, I'm still catching up from being OOO. I'll test tomorrow.

@kbecciv - to confirm, is this bug within the native app or when using Safari on the iPhone? These two points need to be clarified (see screenshot). I tried testing but need to confirm if I need to be using the iPhone application or the app via a mobile browser. Thanks!

Waiting for the updated testing steps

@alexpensify Issue is occurring on only IOS native, updated the step 1.

Thanks for confirming @kbecciv - I'll test using iOS native.

I'm having trouble with BrowserStack, I'll try testing again soon.

Tom helped me test on the native app and was able to replicate it.

Proposal
Contributor details
Your Expensify account email<EMAIL_ADDRESS>Upwork Profile Link: https://www.upwork.com/freelancers/~014ecc4269a1dbeca0
Please re-state the problem that we are trying to solve in this issue.
If the multiline input has a fixed height (because of the screen size) and the lines are more than what can be visible on the screen, when scrolling up to the first line and setting the cursor to the end of the line then pressing Enter for a new line, the input component will scroll down and hide the first line and the new line.
https://github.com/Expensify/App/assets/10665376/b52e1f95-c9b0-4ef5-b396-7bde8936b21e
What is the root cause of that problem?
Rerendering the text input component by changing the text value to the state draft value makes unintended changes to the selection object. Here
It replaces the value of the input which leads to scrolling to the end of text. This changes the selection value 3 times very quickly while having the second position value set to the end of the draft, which makes the text input component scroll/jump to catch up with the selection position changes and ends up scrolling to a random offset.
What changes do you think we should make in order to solve the problem?
You should stop updating the text input value of the text input using the draft value saved in the component state. Instead, you can set the initial draftMessage as private property in the class constructor and replace the value prop of the Composer with defaultValue setting it to the initial draftMessage and it should just work fine.
https://github.com/Expensify/App/assets/10665376/78dedb6c-04e0-411a-8409-eb3ba2618b22
What alternative solutions did you explore? (Optional)
N/A

Proposal
Please re-state the problem that we are trying to solve in this issue.
If the multiline input has a fixed height (because of the screen size) and the lines are more than what can be visible on the screen, when scrolling up to the first line and setting the cursor to the end of the line then pressing Enter for a new line, the input component will scroll down and hide the first line and the new line.
https://github.com/Expensify/App/assets/10665376/b52e1f95-c9b0-4ef5-b396-7bde8936b21e
What is the root cause of that problem?
Rerendering the text input component by changing the text value to the state draft value makes unintended changes to the selection object. Here
It replaces the value of the input which leads to scrolling to the end of text. This changes the selection value 3 times very quickly while having the second position value set to the end of the draft, which makes the text input component scroll/jump to catch up with the selection position changes and ends up scrolling to a random offset.
What changes do you think we should make in order to solve the problem?
You should stop updating the text input value of the text input using the draft value saved in the component state. Instead, you can set the initial draftMessage as private property in the class constructor and replace the value prop of the Composer with defaultValue setting it to the initial draftMessage and it should just work fine.
https://github.com/Expensify/App/assets/10665376/78dedb6c-04e0-411a-8409-eb3ba2618b22
What alternative solutions did you explore? (Optional)
N/A

Waiting for proposals

Proposal
Please re-state the problem that we are trying to solve in this issue.
If the multiline input has a fixed height (because of the screen size) and the lines are more than what can be visible on the screen, when scrolling up to the first line and setting the cursor to the end of the line then pressing Enter for a new line, the input component will scroll down and hide the first line and the new line.
https://github.com/Expensify/App/assets/10665376/b52e1f95-c9b0-4ef5-b396-7bde8936b21e
What is the root cause of that problem?
Rerendering the text input component by changing the text value to the state draft value makes unintended changes to the selection object. (Here)
It replaces the value of the input which leads to scrolling to the end of text. This changes the selection value 3 times very quickly while having the second position value set to the end of the draft, which makes the text input component scroll/jump to catch up with the selection position changes and ends up scrolling to a random offset.
What changes do you think we should make in order to solve the problem?
You should stop updating the text input value of the text input using the draft value saved in the component state. Instead, you can set the initial draftMessage as private property in the class constructor and replace the value prop of the Composer with defaultValue setting it to the initial draftMessage and it should just work fine.
https://github.com/Expensify/App/assets/10665376/78dedb6c-04e0-411a-8409-eb3ba2618b22
What alternative solutions did you explore? (Optional)
N/A

@eVoloshchak - when you get a chance, please review the recent proposal. Thank you!

Huh, that is an interesting issue
Thanks for the bump, I did miss the one on the weekend!
@khaterdev's proposal looks promising, but I'm not 100% sure just yet and wanna do some testing. I'll get back with a review tomorrow

I don't think triggerSaveOrCancel has anything to do with the issue. It shouldn't be called when you press 'enter' key on iOS keyboard.

That was a type, I meant (this) line.

Can confirm this issue happening. The updateDraft function is called twice. The main question is: Why is this happening? What's causing this? Why can't we use state as we currently are? Would this still be reproducible if we migrate this to a functional component? In short, I think we should figure out what exactly is the root cause of this

I tested it with a new blank application. Also, used functional components. It does have the same behavior. So, it is not related to any of the Expensify codes.
Maybe it is a bug in the TextInput component itself or intended behavior. But, it looks like it is rendering the component on state change by setting the value prop to the state value, then this, somehow, might be triggering the onChangeText event, which in turn tries to set the state again (Here). Then it re-renders and acts like replacing the text manually, which leads to putting the cursor at the end of the text. The whole thing changes the cursor position twice very quickly. The change should make the TextInput scroll and changing it twice makes it scroll to 2 different positions which ends on a random position.

I don't think triggerSaveOrCancel has anything to do with the issue. It shouldn't be called when you press 'enter' key on iOS keyboard.

That was a type, I meant (this) line.

Can confirm this issue happening. The updateDraft function is called twice. The main question is: Why is this happening? What's causing this? Why can't we use state as we currently are? Would this still be reproducible if we migrate this to a functional component? In short, I think we should figure out what exactly is the root cause of this

I tested it with a new blank application. Also, used functional components. It does have the same behavior. So, it is not related to any of the Expensify codes.
Maybe it is a bug in the TextInput component itself or intended behavior. But, it looks like it is rendering the component on state change by setting the value prop to the state value, then, somehow, it might be triggering the onChangeText event, which in turn tries to set the state again (Here). Then it re-renders and acts like replacing the text manually, which leads to putting the cursor at the end of the text. The whole thing changes the cursor position twice very quickly. The change should make the TextInput scroll and changing it twice makes it scroll to 2 different positions which ends on a random position.

Looks like the official React Native documentation site is already showing it the same way I suggested in my proposal. It uses the defaultValue prop to set the initial value and the onChangeText to update the state. It is not showing updating the value of the TextInput component using the state again as it will lead to re-rending the component.
Looks like the official React Native documentation site is already showing it the same way I suggested in my proposal. It uses the defaultValue prop to set the initial value and the onChangeText to update the state. It is not showing updating the value of the TextInput component using the state again as it will lead to re-rending the component.

I don't think triggerSaveOrCancel has anything to do with the issue. It shouldn't be called when you press 'enter' key on iOS keyboard.

That was a type, I meant (this) line.

Can confirm this issue happening. The updateDraft function is called twice. The main question is: Why is this happening? What's causing this? Why can't we use state as we currently are? Would this still be reproducible if we migrate this to a functional component? In short, I think we should figure out what exactly is the root cause of this

I tested it with a new blank application. Also, used functional components. It does have the same behavior. So, it is not related to any of the Expensify codes.
Maybe it is a bug in the TextInput component itself or intended behavior. But, it looks like it is rendering the component on state change by setting the value prop to the state value, then, somehow, it might be triggering the onChangeText event, which in turn tries to set the state again (Here). Then it re-renders and acts like replacing the text manually, which leads to putting the cursor at the end of the text. The whole thing changes the cursor position twice very quickly. The change should make the TextInput scroll and changing it twice makes it scroll to 2 different positions which ends on a random position.
Looks like the official React Native documentation site is already showing it the same way I suggested in my proposal. It uses the defaultValue prop to set the initial value and the onChangeText to update the state. It is not showing updating the value of the TextInput component using the state again as it will lead to re-rending the component.

Maybe it is a bug in the TextInput component itself or intended behavior.

In that case, we should fix it in react-native itself.
If there's abug in react-native, this means the same issue might appear on other pages too, so an upstream fix would be better than refactoring every page. Could you share a snack with a blank application with minimal reproduction steps for this? Are there any open react-native issues related to this?

Maybe it is a bug in the TextInput component itself or intended behavior.

In that case, we should fix it in react-native itself. If there's a bug in react-native, this means the same issue might appear on other pages too, so an upstream fix would be better than refactoring every page. Could you share a snack with a blank application with minimal reproduction steps for this? Are there any open react-native issues related to this?

After checking again, I suspect that it might not be a bug in react-native as setting the value prop is expected to re-render the component while it was already re-rendered by changing the state using the onChangeText event. So, I see now it is already expected to re-render twice which leads to the current issue. Again, I suggest using defaultValue to set the initial value instead of setting the value prop on state change. The TextInput component already keeps the entered text without the need of resetting it every time the text changes. We just need to keep the onChangeText event to use the state value during submission.
Here is a snack showing two components, one using the same ideas as the current Expensify code, and the other using my suggestion. You can easily see that the issue only occurs while using the first component only.

Maybe it is a bug in the TextInput component itself or intended behavior.

In that case, we should fix it in react-native itself. If there's a bug in react-native, this means the same issue might appear on other pages too, so an upstream fix would be better than refactoring every page. Could you share a snack with a blank application with minimal reproduction steps for this? Are there any open react-native issues related to this?

After checking again, I suspect that it might not be a bug in react-native as setting the value prop is expected to re-render the component while it was already re-rendered by changing the state using the onChangeText event. So, I see now it is already expected to re-render twice which leads to the current issue. Again, I suggest using defaultValue to set the initial value instead of setting the value prop on state change. The TextInput component already keeps the entered text without the need of resetting it every time the text changes. We just need to keep the onChangeText event to use the state value during submission.
Here is a snack showing two components, one is setting the value prop to the state value as the current Expensify code, and the other using my suggestion by setting the defaultValue prop one time on first render. You can easily see that the issue only occurs while using the first component only.

So knowing what we know now, how would this change your proposal, if at all, @khaterdev?

I don't think this would change anything. I still suggest the same solution mentioned in the proposal. We shouldn't update the value prop using the state. No need to do that. Only the state should be updated if the text is changed so we get only 1 re-render, and pass the initial value to the component using the defaultValue prop.

@deetergp should we update the description of this GH?

@alexpensify @deetergp @eVoloshchak, do you still have questions about this GH? Will we go forward?

@deetergp, new documentation also has a controlled input section.
My point is there's nothing inherently wrong with using value and onChange (i.e. controlled input). Using an uncontrolled input would resolve the issue, but we aren't fixing the actual cause.

Using an uncontrolled input would resolve the issue, but we aren't fixing the actual cause.

@eVoloshchak, isn't this trying to fix an issue you don't have? If you don't need to use a controlled input (for this specific case) and the application works perfectly without it, why are you trying to use it?
The issue seems to be existing for a new setup too as I mentioned before. I think that it is an expected behavior, but maybe it can be fixed within React Native itself.
So, trying to find a solution for the issue or even fixing React Native if it is a bug while we don't need it implemented in Expensify is just out of scope.

Using an uncontrolled input would resolve the issue, but we aren't fixing the actual cause.

@eVoloshchak, isn't this trying to fix an issue you don't have? If you don't need to use a controlled input (for this specific case) and the application works perfectly without it, why are you trying to use it?
The issue seems to be existing for a new setup too as I mentioned before. I think that it is an expected behavior (at least the re-render part), but maybe it can be fixed within React Native itself.
So, trying to find a solution for the issue or even fixing React Native if it is a bug while we don't need it implemented in Expensify is just out of scope.

If you don't need to use a controlled input (for this specific case) and the application works perfectly without it, why are you trying to use it?

Because the issue will come back in case we ever need to make this input controlled. And all of the controlled multiline inputs (that exist now or will be added in the future) will keep having the same issue

The issue seems to be existing for a new setup too as I mentioned before. I think that it is an expected behavior (at least the re-render part), but maybe it can be fixed within React Native itself.

Fixing it in react-native itself would be the way to go I think

Reproducing the issue on iOS without react-native
CLICK TO OPEN SOURCE CODE

//
//  ViewController.m
//  iPhoneTestApp
//
//  Created by Fabrizio Bertoglio on 5/17/23.
//

#import "ViewController.h"

@interface RCTUITextView : UITextView<UITextViewDelegate>
@end

@implementation RCTUITextView {}

-(void)textViewDidChange:(UITextView *)textView
{
  NSLog(@"textViewDidChange : %@",textView.text);
  NSMutableDictionary<NSAttributedStringKey, id> *attributes = [NSMutableDictionary dictionaryWithCapacity:10];
  attributes[NSForegroundColorAttributeName] = [UIColor blueColor];
  NSAttributedString *attributedString = [[NSAttributedString alloc] initWithString:textView.text attributes: attributes];

  [textView setAttributedText: attributedString];
}

@end

@interface ViewController ()

@end

@implementation ViewController

- (void)viewDidLoad {
  [super viewDidLoad];
  // Do any additional setup after loading the view.
  RCTUITextView *textView = [[RCTUITextView alloc] initWithFrame:CGRectMake(10, 200, 300, 50)];
  textView.scrollEnabled = true;
  textView.delegate = textView;
  UIView *redBackgroundView = [[UIView alloc] initWithFrame:CGRectMake(0, 0, self.view.frame.size.width, self.view.frame.size.height / 2.0)];
  redBackgroundView.backgroundColor = [UIColor redColor];
  [self.view addSubview:redBackgroundView];
  [redBackgroundView addSubview:textView];
}

@end

https://user-images.githubusercontent.com/24992535/246601549-99f480f3-ce80-4678-9378-f71c8aa67e17.mp4

Fixing the issue on iOS without react-native
CLICK TO OPEN TESTS RESULTS

//
//  ViewController.m
//  iPhoneTestApp
//
//  Created by Fabrizio Bertoglio on 5/17/23.
//

#import "ViewController.h"

@interface RCTUITextView : UITextView<UITextViewDelegate>
@end

@implementation RCTUITextView {}

-(void)textViewDidChange:(UITextView *)textView
{
  // disable scroll to avoid issue #
  textView.scrollEnabled = false;
  NSLog(@"textViewDidChange : %@",textView.text);
  NSMutableDictionary<NSAttributedStringKey, id> *attributes = [NSMutableDictionary dictionaryWithCapacity:10];
  attributes[NSForegroundColorAttributeName] = [UIColor blueColor];
  NSAttributedString *attributedString = [[NSAttributedString alloc] initWithString:textView.text attributes: attributes];

  // save cursor position
  UITextRange *selectedRange = self.selectedTextRange;
  NSInteger oldTextLength = self.attributedText.string.length;

  [textView setAttributedText: attributedString];

  // restore cursor position
  if (selectedRange.empty) {
    // Maintaining a cursor position relative to the end of the old text.
    NSInteger offsetStart = [self offsetFromPosition:self.beginningOfDocument
                                          toPosition:selectedRange.start];
    NSInteger offsetFromEnd = oldTextLength - offsetStart;
    NSInteger newOffset = textView.text.length - offsetFromEnd;
    UITextPosition *position = [self positionFromPosition:self.beginningOfDocument
                                                   offset:newOffset];
    [self setSelectedTextRange:[self textRangeFromPosition:position toPosition:position]];
  }

  textView.scrollEnabled = true;
}

@end

@interface ViewController ()

@end

@implementation ViewController

- (void)viewDidLoad {
  [super viewDidLoad];
  // Do any additional setup after loading the view.
  RCTUITextView *textView = [[RCTUITextView alloc] initWithFrame:CGRectMake(10, 200, 300, 50)];
  textView.scrollEnabled = true;
  textView.delegate = textView;
  UIView *redBackgroundView = [[UIView alloc] initWithFrame:CGRectMake(0, 0, self.view.frame.size.width, self.view.frame.size.height / 2.0)];
  redBackgroundView.backgroundColor = [UIColor redColor];
  [self.view addSubview:redBackgroundView];
  [redBackgroundView addSubview:textView];
}

@end

https://user-images.githubusercontent.com/24992535/246606574-92da5805-e6b1-4675-b1e7-e97dd494a9bb.mp4

Reproducing the issue in React-Native
CLICK TO OPEN SOURCECODE

- (void)setAttributedText:(NSAttributedString *)attributedText
{
  [super setAttributedText:attributedText];
  [self textDidChange];
}

https://user-images.githubusercontent.com/24992535/246601996-fe08ce4f-6a6f-460d-bb06-da3dd0feaaaf.mp4

Fixing the issue in React-Native
CLICK TO OPEN SOURCECODE

- (void)setAttributedText:(NSAttributedString *)attributedText
{
  self.scrollEnabled = false;
  [super setAttributedText:attributedText];
  [self textDidChange];
  self.scrollEnabled = true;
}

https://user-images.githubusercontent.com/24992535/246602165-c1bc1b44-087f-4ec9-bc44-14f961eaa4d1.mp4

Proposal
Please re-state the problem that we are trying to solve in this issue.
Multiline TextInput with a fixed height will scroll to the bottom of the screen when prepending new lines to the text.
What is the root cause of that problem?
The issue is caused by iOS UITextView:

The cursor moves to the end of the text when prepending new lines.
Moving the cursor to the end of the text triggers the scroll to the bottom.

The behavior was reproduced on an iOS App (without react-native).
The example included below implements a Component RCTUITextView based on UITextView, which modifies the UITextView attributedText with the textViewDidChange callback (source code available in this comment).
Adding a new line on top of the UITextView on iOS results in:
Issue 1) The cursor moves to the end of TextInput text
Issue 2) The TextInput scrolls to the bottom
Reproducing the issue on an iOS App without react-native

Issue 1) is already fixed in react-native, which restores the previous cursor position (on Fabric with  _setAttributedString) after changing the text.
Issue 2) needs to be fixed in react-native.
What changes do you think we should make in order to solve the problem?
The functionality was fixed on the iOS example with the following steps (source code available in this comment):
Issue 1) implementing logic to restore the previous cursor position as done in React Native Fabric _setAttributedString
Issue 2) disable scrollingEnabled when changing the attributedText
Fixing the issue on iOS without react-native

The solution was tested in react-native and fixed the issue as documented below.
Issue 1) already fixed
Issue 2) fixed by disabling scrollingEnabled while changing the attributedText
Reproducing the issue in React-Native

Fixing the issue in React-Native

What alternative solutions did you explore? (Optional)
I did not investigate an alternative solution.

@eVoloshchak - when you get a chance, can you review the recent proposals? Thank you!

@fabriziobertoglio1987, wow, thanks for such a detailed explanation!

implementing logic to restore the previous cursor position as done in React Native Fabric

We're in the progress of migrating to Fabric, so if this part is already resolved there - that's awesome

fixed by disabling scrollingEnabled while changing the attributedText

I've tested and it does resolve the issue. However, we still need to get to the bottom of why the issue is happening. According to your comment, this is a bug with iOS itself. Could you please take a look if there are any issues/bug reports for iOS on this?
I found https://github.com/facebook/react-native/issues/30207, which I think is related

Thank you @eVoloshchak! @fabriziobertoglio1987 - keep us posted on your findings or questions to the feedback provided in the last comment.

Thanks a lot. I further investigated this issue and this is my update.

According to your comment, this is a bug with iOS itself. Could you please take a look if there are any issues/bug reports for iOS on this?

Commit Maintain cursor position when the text changes in the text input was introduced to fix iOS issue 652653.
https://developer.apple.com/forums/thread/652653
https://stackoverflow.com/questions/34914948/how-to-stop-cursor-changing-position-when-i-setattributedtext-in-uitextview-dele

Updating the UITextView attributedText causes the cursor to move to the end.
Moving the cursor to the end of the text triggers a scroll to the bottom.

However, we still need to get to the bottom of why the issue is happening.

@eVoloshchak is correct, the root cause of this issue is in react-native itself.

The issue is caused by adding new line character (\n) in the text, but does not reproduce with other characters.
Reproduces with controlled and not controlled TextInput components in Fabric

The root cause is:

_textOf fails to compare identical strings. The equality comparison between oldText "1\n\n2\n3\n" and newText "1\n\n2\n3\n" returns false.
Triggers an additional update of the attributedText, replacing it with an identical string. The UITextView already includes the new line, but _textOf fails to compare the current UITextView state (_backedTextInputView.attributedText with value "1\n\n2\n3\n") with the next state (attributedString with value "1\n\n2\n3\n").
The additional update causes the cursor to move to the end of the text (iOS issue 652653)
Moving the cursor to the end of the text triggers a scroll to the bottom.

The root cause of this issue is STEP 1) ( _textOf failed to compare identical strings).
I tested a temporary fix for the issue. I will update my proposal in a couple of days with the final solution.
The solution was only developed for testing purposes, I will update the solution soon.
Reproducing the issue on React Native not controlled TextInput

https://user-images.githubusercontent.com/24992535/247505358-4b10fc23-a0fd-4c0a-b06f-fd17bf671d77.mp4

Fixing the issue on React Native

Not Controlled Component

https://user-images.githubusercontent.com/24992535/247506063-0eaabccc-bc8f-4883-ba2f-8e5bfc1bc98e.mp4

Controlled Component

https://user-images.githubusercontent.com/24992535/247507038-a95be425-3373-4d1e-bee4-bc2eb6241434.mp4

@eVoloshchak - I think we are making progress here but let me know if we should move to internal.

@alexpensify, yeah, there's progress being made by @fabriziobertoglio1987
I think we should add the External and Help Wanted labels back

Done, thank you! @eVoloshchak is there any remaining feedback for @fabriziobertoglio1987 based on their latest reply?

I further investigated the issue. This is my update for today.
I was able to reproduce the same issue with a bare iOS iPhone App with the following steps:

The iOS App renders a UITextView
At initialization, the UITextView AttributedString foreground color is set to red
Typing a new line triggers textDidChangeEvent
The UITextView foreground color changes to green on textDidChangeEvent
The update causes the cursor to move to the end of the string
Moving the cursor to the end of the string, causes the TextView to scroll to the end

The issue reproduces on the iOS App without react-native only when creating a new AttributedString, while it does not reproduce when using a mutable copy of the AttributedString.
Changing the foreground color of the AttributedString using a mutable copy of the original AttributedString does not scroll the TextView to the bottom

https://user-images.githubusercontent.com/24992535/248256843-55673df8-2bfe-4cfe-98cb-ed3a3bff0317.mp4

Creating a new AttributedString with a different foreground color causes the cursor to jump to the end of the line.

https://user-images.githubusercontent.com/24992535/248256498-a556fd65-a75a-4902-9010-f17fc88ac0da.mp4

Click to open sourcecode

//
//  ViewController.m
//  iPhoneTestApp
//
//  Created by Fabrizio Bertoglio on 5/17/23.
//

#import "ViewController.h"

@interface RCTUITextView : UITextView<UITextViewDelegate>
@end

@implementation RCTUITextView {
  NSAttributedString *_oldAttributedString;
  NSMutableDictionary<NSAttributedStringKey, id> *_attributes;
}

-(void)textViewDidChange:(UITextView *)textView
{
 /*
  NSMutableDictionary<NSAttributedStringKey, id> *attributes = [NSMutableDictionary dictionaryWithCapacity:10];
  attributes[NSForegroundColorAttributeName] = [UIColor greenColor];
  NSAttributedString *newAttributes = [[NSAttributedString alloc] initWithString:textView.text attributes:attributes];
  [textView setAttributedText: newAttributes];
 */

  NSMutableAttributedString *attributedStringCopy = [textView.attributedText mutableCopy];
  [attributedStringCopy addAttributes:@{ NSForegroundColorAttributeName : [UIColor greenColor]} range:NSMakeRange(0, attributedStringCopy.length)];
  [textView setAttributedText: attributedStringCopy];
}

@end

@interface ViewController ()

@end

@implementation ViewController

- (void)viewDidLoad {
  [super viewDidLoad];
  // Do any additional setup after loading the view.
  RCTUITextView *textView = [[RCTUITextView alloc] initWithFrame:CGRectMake(10, 200, 300, 50)];
  textView.scrollEnabled = true;
  textView.delegate = textView;

  NSMutableDictionary<NSAttributedStringKey, id> *attributes = [NSMutableDictionary dictionaryWithCapacity:10];
  attributes[NSForegroundColorAttributeName] = [UIColor redColor];
  NSAttributedString *attributedString = [[NSAttributedString alloc] initWithString:textView.text attributes: attributes];
  [textView setAttributedText:attributedString];

  UIView *redBackgroundView = [[UIView alloc] initWithFrame:CGRectMake(0, 0, self.view.frame.size.width, self.view.frame.size.height / 2.0)];
  redBackgroundView.backgroundColor = [UIColor redColor];
  [self.view addSubview:redBackgroundView];
  [redBackgroundView addSubview:textView];
}

@end

I will further work on this issue and update the proposal soon. Thanks a lot for your attention.

Thank you for that update @fabriziobertoglio1987

@eVoloshchak - could you review again for feedback? Thank you!

Will post a review tomorrow, this one is an interesting issue to research (Apple bugs are fun), thanks for the investigation @fabriziobertoglio1987

@eVoloshchak - any update here on the review status? Thank you!

Can reproduce this on a bare project (without react-native), thank you @fabriziobertoglio1987 for those examples

This is a bug in iOS (is there a ticket on Apple's tracker for this?)
This was partially fixed by https://github.com/facebook/react-native/commit/b0e39b2ed9b66b378eb75bee9e692fc801431ddd in Fabric
Disabling scrolling when changing the attributedText fixes the issue, but this is a workaround
I'm fairly certain Apple won't fix this any time soon

We should avoid workarounds and fix the root cause instead. On the other hand, this is a bug with iOS itself which we can't fix, and Apple won't fix it probably. However, in my opinion, the bug isn't severe enough to justify the use of a workaround solution right away.
I think we should wait and seek a better solution, only if there is no other option we should apply a workaround
Either way, a good place to start would be to

open a ticket for this bug on Apple's forums (if there isn't one already)
open an issue for this in react-native repo (if there isn't one already)

Reassigning another bug team member, I'm going OOO until Monday, July 10, and will take it back then if it's not closed by then.
Required action from the 🐛 team: Make sure the proposal process is moving forward, so we can get this one assigned out to fix. Also, we might have to start a discussion if we should close.

The root cause of the Issue

Prepending a new line with \n causes a change in the AttributedString
Changing the AttributedString attributes causes the cursor to move to the end of the string
Moving the cursor to the end of the string causes the TextView to scroll to the end

Changing any field of the AttributedString causes the scroll to the end (more info in https://github.com/Expensify/App/issues/19507#issuecomment-1607559935).
Prepending \n triggers an update of the AttributedString attributes which applies the following changes:

old AttributedString has 8 blocks, string "1\n\n2", and a total length of 5. 3 blocks (spans) are used and have the following styles.

Printing description of oldAttributedString->attributes:
Length 5 (8 blocks, 3 used, block 0 is at 0)
 2 0x600002e70230 {
    NSBackgroundColor = "UIExtendedGrayColorSpace 0 0";
    NSColor = "UIExtendedGrayColorSpace 0 1";
    NSFont = "<UICTFont: 0x130457dc0> font-family: \".SFUI-Regular\"; font-weight: normal; font-style: normal; font-size: 14.00pt";
    NSParagraphStyle = "Alignment 4, LineSpacing 0, ParagraphSpacing 0, ParagraphSpacingBefore 0, HeadIndent 0, TailIndent 0, FirstLineHeadIndent 0, LineHeight 0/0, LineHeightMultiple 0, LineBreakMode 0, Tabs (\n    28L,\n    56L,\n    84L,\n    112L,\n    140L,\n    168L,\n    196L,\n    224L,\n    252L,\n    280L,\n    308L,\n    336L\n), DefaultTabInterval 0, Blocks (\n), Lists (\n), BaseWritingDirection 0, HyphenationFactor 0, TighteningForTruncation NO, HeaderLevel 0 LineBreakStrategy 0 PresentationIntents (\n) ListIntentOrdinal 0 CodeBlockIntentLanguageHint ''";
}
 1 0x600002f9d5e0 {
    NSBackgroundColor = "UIExtendedGrayColorSpace 0 0";
    NSColor = "UIExtendedGrayColorSpace 0 1";
    NSFont = "<UICTFont: 0x130457dc0> font-family: \".SFUI-Regular\"; font-weight: normal; font-style: normal; font-size: 14.00pt";
    NSParagraphStyle = "Alignment 0, LineSpacing 0, ParagraphSpacing 0, ParagraphSpacingBefore 0, HeadIndent 0, TailIndent 0, FirstLineHeadIndent 0, LineHeight 0/0, LineHeightMultiple 0, LineBreakMode 0, Tabs (\n    28L,\n    56L,\n    84L,\n    112L,\n    140L,\n    168L,\n    196L,\n    224L,\n    252L,\n    280L,\n    308L,\n    336L\n), DefaultTabInterval 0, Blocks (\n), Lists (\n), BaseWritingDirection 0, HyphenationFactor 0, TighteningForTruncation NO, HeaderLevel 0 LineBreakStrategy 0 PresentationIntents (\n) ListIntentOrdinal 0 CodeBlockIntentLanguageHint ''";
}
 2 0x600002e70230 {
    NSBackgroundColor = "UIExtendedGrayColorSpace 0 0";
    NSColor = "UIExtendedGrayColorSpace 0 1";
    NSFont = "<UICTFont: 0x130457dc0> font-family: \".SFUI-Regular\"; font-weight: normal; font-style: normal; font-size: 14.00pt";
    NSParagraphStyle = "Alignment 4, LineSpacing 0, ParagraphSpacing 0, ParagraphSpacingBefore 0, HeadIndent 0, TailIndent 0, FirstLineHeadIndent 0, LineHeight 0/0, LineHeightMultiple 0, LineBreakMode 0, Tabs (\n    28L,\n    56L,\n    84L,\n    112L,\n    140L,\n    168L,\n    196L,\n    224L,\n    252L,\n    280L,\n    308L,\n    336L\n), DefaultTabInterval 0, Blocks (\n), Lists (\n), BaseWritingDirection 0, HyphenationFactor 0, TighteningForTruncation NO, HeaderLevel 0 LineBreakStrategy 0 PresentationIntents (\n) ListIntentOrdinal 0 CodeBlockIntentLanguageHint ''";
}

The new AttributedString has 2 blocks, 1 used (1 span), string is "1\n\n2". The total length is 5 blocks.

Printing description of attributedString->attributes:
Length 5 (2 blocks, 1 used, block 0 is at 0)
 5 0x600002e70230 {
    NSBackgroundColor = "UIExtendedGrayColorSpace 0 0";
    NSColor = "UIExtendedGrayColorSpace 0 1";
    NSFont = "<UICTFont: 0x130457dc0> font-family: \".SFUI-Regular\"; font-weight: normal; font-style: normal; font-size: 14.00pt";
    NSParagraphStyle = "Alignment 4, LineSpacing 0, ParagraphSpacing 0, ParagraphSpacingBefore 0, HeadIndent 0, TailIndent 0, FirstLineHeadIndent 0, LineHeight 0/0, LineHeightMultiple 0, LineBreakMode 0, Tabs (\n    28L,\n    56L,\n    84L,\n    112L,\n    140L,\n    168L,\n    196L,\n    224L,\n    252L,\n    280L,\n    308L,\n    336L\n), DefaultTabInterval 0, Blocks (\n), Lists (\n), BaseWritingDirection 0, HyphenationFactor 0, TighteningForTruncation NO, HeaderLevel 0 LineBreakStrategy 0 PresentationIntents (\n) ListIntentOrdinal 0 CodeBlockIntentLanguageHint ''";
}

The old AttributedString block 1 has NSParagraphStyle Alignment 0
The new AttributedString block 5 has NSParagraphStyle Alignment 4

https://user-images.githubusercontent.com/24992535/250248022-27665b9c-bd23-403f-819e-a9ca438d3350.mp4
Alternative Solution - Avoid updating AttributedString without relevant changes in the text or attributes
The alternative solution is detecting this scenario based on the following conditions:

The two strings are identical (the old string is "1\n\n2" and the new string is "1\n\n2")
No relevant changes in attributes (in the case above all attributes are identical, but they are applied using 3 spans instead of one span).

As there are no relevant changes in the attributes or in the string, avoid updating the NSAttributedString, which causes the TextView to scroll to the end (https://github.com/Expensify/App/issues/19507#issuecomment-1600545816).

@eVoloshchak how does @fabriziobertoglio1987 's updated proposal look to you?

Prepending a new line with \n causes a change in the AttributedString
Changing the AttributedString attributes causes the cursor to move to the end of the string
Moving the cursor to the end of the string causes the TextView to scroll to the end

@fabriziobertoglio1987, awesome! Is there an issue for this on Apple's forums? Or in react-native repository?

The alternative solution is detecting this scenario based on the following conditions:
The two strings are identical (the old string is "1\n\n2" and the new string is "1\n\n2")
No relevant changes in attributes (in the case above all attributes are identical, but they are applied using 3 spans instead of one span).

This is a workaround. I suspect Apple won't fix this, so we'll probably end up implementing a workaround as a last resort. But before that, I think we should try and find if there is any discussion/bug reports on this

Were you able to find any open issues in the Apple forums, @fabriziobertoglio1987?

Alright, removing the other 🐛 team member. I'm back online and see that this is our current state:
Waiting on a reply to @deetergp's question

Were you able to find any open issues in the Apple forums

Thanks @deetergp. I'm currently working on other 2 issues, sorry for my quick reply.

The Apple issue tracker is not public, bug reports are filed privately to Apple https://developer.apple.com/forums/thread/712889
There is an open issue in the Apple Forums and the solution is in this StackOverflow discussion. The solution was implemented in react-native with commit https://github.com/facebook/react-native/commit/b0e39b2ed9b66b378eb75bee9e692fc801431ddd (more info in this comment).

@eVoloshchak https://github.com/Expensify/App/issues/19507#issuecomment-1597488640

I found https://github.com/facebook/react-native/issues/30207, which I think is related

https://developer.apple.com/documentation/uikit/uiscrollview?language=objc

UIScrollView is the superclass of several UIKit classes, including UITableView and UITextView.

I found these reports on the Apple forums, but did not yet have the time to reproduce and debug those issues on iOS.

https://developer.apple.com/forums/thread/691446
https://developer.apple.com/forums/thread/662056

Thank you for this update @fabriziobertoglio1987!

So basically they have implemented the workaround from the Apple forum issue in React Native proper? Nice!
Let me restate what you said in your previous comment to make sure I am understanding it correctly. In order to fix this bug (whose bug status is debatable since it's a default behavior for the platform) we need to:

If we implement a version of react-native that contains the linked commit, it should take care of re-positioning the cursor when the text has NOT changed.
In the instances where the text has changed, we would need to implement a workaround where we disable scrolling (just for IOS).

But, according to our README's Philosophy section 5.ii:

If the reason you can't write cross-platform code is because there is a bug in ReactNative that is preventing it from working, the correct action is to fix RN and submit a PR upstream -- not to hack around RN bugs with platform-specific code paths.

It sounds like we should be trying to implement this workaround upstream in react-native.

Thanks @deetergp

It sounds like we should be trying to implement this workaround upstream in react-native.

My solution is implementing the fix upstream in react-native (proposal). I reproduced the issue in facebook/react-native and tested the fix in the videos below. I will update my proposal soon, to make it more clear.

Before applying the fix to facebook/react-native

After applying the fix to facebook/react-native

Thank you for that update!

@deetergp could you review the latest update here, thanks

@deetergp any update here?

@deetergp is OOO this week. I'm going to see if I can get another engineer to review it.

I ran out of time yesterday, I'll shop around on Monday.

@dangrous - @deetergp was OOO and working limited hours this week. We need an engineer to review @fabriziobertoglio1987 feedback here: https://github.com/Expensify/App/issues/19507#issuecomment-1635281369. Thanks!

Hi! Reading back through this, it seems like the migration to Fabric as well as the (pretty minor) change @fabriziobertoglio1987 suggests will solve this problem. I'm not quite sure on the next steps though - @fabriziobertoglio1987 were you going to submit a PR directly to facebook/react-native? Or were we going to put it in our fork as well? I haven't dealt with an upstream RN fix yet, sorry!

@dangrous, I think the standard process for this would be to do both: submit a PR to facebook/react-native as well as put it in our fork so we can fix the bug right away

k, let's do it.  Thank you!

Update 26/07
Thanks a lot! I'm working on another task this week.
I would start working on this task next week. I plan to publish PR between the 14-18th of August.

@fabriziobertoglio1987, keep in mind that according to CONTRIBUTING.md

Payment timeline all based on the day and timestamp the contributor has an accepted proposal and is assigned to the Github issue
Merged PR within 3 business days (72 hours) - 50% bonus
Merged PR within 6 business days (144 hours) - 0% bonus
Merged PR within 9 business days (216 hours) - 50% penalty
No PR within 12 business days - Contract terminated

I think this applies only to the PR in our fork of react-native, since PR in facebook/react-native is out of our control

Yeah we'd still like to stick to that timeline for our fork - do you think there's a way you could get to it sooner, @fabriziobertoglio1987? If not we should probably go ahead and open this one back up, since as @eVoloshchak noted 8/14-8/18 will be outside of even the 12 day timeline.
Thanks!

Update 27/07
Sorry @dangrous @eVoloshchak. Yes. I can publish the PR earlier.
I would publish the PR between the 2nd and the 9th of August. Would that be okay? Thanks

Hi @fabriziobertoglio1987, thanks for the update! Given that that is still pretty far in the future, we're going to go ahead and unassign you, in case there's someone available who'd like to give this issue a shot in the meantime - but if the issue is still available once you're free to work on it, feel free to ping and we can get you assigned again!

Accepting proposals again here

@dangrous @alexpensify I already accepted the upwork job. I will publish the PR today. Thanks a lot

Update 29/07
Published PRs https://github.com/facebook/react-native/pull/38679 and https://github.com/Expensify/react-native/pull/60.

Oh, amazing! @eVoloshchak do you want to give the PR in our fork a look (I can't assign you directly) and then I can check it out as well?
@alexpensify let me know if anything weird will happen if I reassign @fabriziobertoglio1987

@fabriziobertoglio1987 was assigned by me over the weekend.

Oh duh, I should have looked more closely at the sidebar haha

All good!
Looks like the PRs are moving along!

PR is merged into our fork, though I've learned we're getting rid of our fork haha.
We'll just have to wait for FB to merge the upstream PR, which is out of our hands. We should track the timing based off of this PR though!

Gonna switch this to weekly until we hear about the other PR, since there's literally nothing any of us can do.

Weekly update: On hold for the FB update

Weekly update: On hold for the FB update, which is moving

Update 19/08
Completed changes requested in the code review. I will move the PR to ready for review on Monday 21st of August.
https://github.com/facebook/react-native/assets/24992535/14811724-8dc7-40f9-824e-713ff14c45df

Update 20/08
I moved the PR to ready to review.

Weekly Update, it looks like the FB update is moving along and almost to the finish line.

Weekly Update: It looks like the FB PR is blocked. Is there any action we can take to unblock it or continue to hold?

Update 31/08
I contacted the maintainer and I will collaborate with him until the PR is merged (relevant comment).

Thanks for that @fabriziobertoglio1987! Looks like we're close, based on the reviewer's last comment

Weekly update: FB is really close!

@fabriziobertoglio1987 do you maybe want to check in with the reviewer? No worries if you don't feel comfortable, but it's been a bit of time since their latest update. Thanks!

Thanks @dangrous. No problem. I'll reach out and follow up. I'll be available 2-3 days a week from the 18th of September till the 18th of October because I'm renovating my new home.

Weekly update: Waiting for FB to respond
Good luck with the renovations @fabriziobertoglio1987!

Weekly update: Waiting for FB to bring in some specialist to review

Same as above, bu updates in https://github.com/facebook/react-native/pull/38679 yet

Thank you @eVoloshchak for the update

Weekly update: Waiting for FB

Weekly update: Waiting for FB

@fabriziobertoglio1987 do you mind reaching out to FB on that issue again, to see if we can get it moving?

Thanks a lot @dangrous . I contacted the FB Team (https://github.com/facebook/react-native/pull/38679#issuecomment-1774322090) and I will help them to merge the PR in the main branch.
Now I'm waiting for their feedback. I will be again available from the 28th of October.

Thank you @eVoloshchak for the update!

No updates, still waiting for FB

I'm back and I'm happy to help get the PR merged into the main branch. Thanks

Thanks! Keep us posted @fabriziobertoglio1987.

Weekly Update: Still on hold because of FB

Weekly Update: Still on hold because of FB

Weekly Update: Still on hold because of FB

Weekly Update: Still on hold because of FB

Weekly Update: Still on hold because of FB

Update 17/12/2023

Updated branch based on the latest main and tested functionalities
I will request code review from the react-native team during the working weekdays

Thanks

Flagging that I will be OOO until January 4. I'll check then for a weekly review.

yeah we're still waiting on FB here

Still no response from FB, so I guess we just chill here...

Hey @roryabraham this is reliant on an upstream PR to the react-native repository... Is there anything we (or one of our expert contributor agencies) can do to get this moving along? It's literally been months of nothing, unfortunately.

@dangrous good call to ask if we get our partners involved here. Thank you!

I can include the patch in the https://github.com/fabOnReact/react-native-improved library.
I patch the native iOS/Android ReactTextView/ReactEditText APIs, but without building from source.
There will be no issues upgrading. My library creates new native APIs based on ReactTextView, ReactEditText etc...
I don't modify JS/Objc/Java files with patch-package.
It does not require building from source react-native and the build will not be slow.
It is the suggested approach from the react-native core team (see this discussion):

The Components are based on ReactTextView/ReactEditText/ReactScrollView.
The library includes patch fixes for react-native and third-party libraries.
The patch are applied only until PR is merged, otherwise they will be automatically removed.

The advantages are:

Increased engagement with the community, issues are quickly fixed (days instead of years)
No limitations when merging PRs (react-native main branch is used on facebook marketplace).
Allows us to further experiment and develop react-native.

Thanks a lot 🙏

@dangrous can we get your feedback on the recent update? Thanks!

Hey @fabOnReact - thanks for the suggestion! I've bothered Callstack about getting the existing PR merged to react native, and they should be able to hopefully handle this week - especially since that lint error seems to be fixed now. I think we can stick to that library for now rather than switching to a new one. Stay tuned!

@dangrous any luck with the update from our partner? Thanks!

Hrm, let me see if I can get an ETA! They're currently prioritizing other things but they said they'd reach out to Nick, the reviewer

Callstack's attempting to help! https://github.com/facebook/react-native/pull/38679#issuecomment-1920864833 I think they're also backchanneling to see if another RN reviewer can take over from Nick

Weekly Update: #Progress! Thanks for the news!

Finally got some follow up questions from FB on the PR, @fabOnReact!

Awesome, thanks @dangrous for this weekly update. @fabOnReact keep us posted if you need more time to review the questions.

Thanks. I look into it this weekend.

Weekly Update: Waiting for feedback to the PR comments

Weekly Update: There was movement in the PR last week and it looks like there are some required tests being prepared to keep moving forward.

Weekly Update: Still waiting on the tests

Weekly Update: There does not appear to be much movement on the PR since the last weekly update.

It looks like the PR we've been holding on was merged last week 🎉

@deetergp or @dangrous - following this FB news, are there any more actions here to complete?

So it's been merged into main on react-native, but we still have to wait for it to actually make it into a release - we can check the tags at the bottom of the commit message here. Once that's done, we need to wait (again, haha) until we start using that react-native version in our app. Unfortunately I think there's still a way to go.
Anyone who knows better, though - let me know if I've got that wrong!

Should we demote this to monthly since "Hurry up and wait" continues to be our next step?

Still waiting.

Update 5 June 2024
I will look into this next week. Thanks

Yeah I'm not seeing this in any release yet, sigh, might be good to check in!

@roryabraham I was wondering - do you have any thoughts on if there's a way to speed this up, or otherwise close out this issue? Short version - @fabOnReact opened a PR to react-native that was merged successfully (or I mean copied and merged, due to their weird process) but it's not in a release yet. Should we create a patch in the meantime? I'm not sure how that process works, I figured you'd be a good person to ask!

I will ask for a cherry pick tomorrow in the upcoming release. Thanks

Update 13 June 2024

Cherry Pick Request for 0.74 https://github.com/reactwg/react-native-releases/issues/314
Status Table for Cherry Pick Requests for 0.74 https://github.com/orgs/reactwg/projects/5/views/3
Instructions on how to do Cherry Pick Requests https://github.com/reactwg/react-native-releases/discussions/117
Expensify Upgrade to 0.74 discussion https://github.com/Expensify/App/issues/37374

Thank you, @fabOnReact, for the update!

okay great! looks like we'll be in 0.75. Progress!

The commit was cherry picked in 0.74. thanks

Update 13 June 2024
cc @dangrous

Cherry Pick Request for 0.74 https://github.com/reactwg/react-native-releases/issues/314
Status Table for Cherry Pick Requests for 0.74 https://github.com/orgs/reactwg/projects/5/views/3
Instructions on how to do Cherry Pick Requests https://github.com/reactwg/react-native-releases/discussions/117
Expensify Upgrade to 0.74 discussion https://github.com/Expensify/App/issues/37374

The commit was cherry picked in 0.74. Thanks

Oh okay awesome! So basically we're just waiting for our own upgrade to RN 0.74?

@dangrous Yes. It was picked in 0.74.3. https://github.com/orgs/reactwg/projects/5/views/3?filterQuery=Multiline
https://github.com/facebook/react-native/blob/0a683e395a72111c35f162d956331634673559e5/packages/react-native/React/Fabric/Mounting/ComponentViews/TextInput/RCTTextInputComponentView.mm#L610

Monthly Update: I think we are still waiting for 0.74

Looks like the 0.74 upgrade is nearly ready to merge!

Updated the title since we're not on hold anymore, just waiting for the merge

I'm not quite sure how long it's going to be... I guess we could update to hold for https://github.com/Expensify/App/issues/37374 instead?

Why is melvin talking about proposals? That's weird. I'll put it on hold for that PR to stop that from happening.

oh wait it's not on staging yet so we can't test yet. But soooooon

on staging I think! Let's test and close out, if we can!

Heads up, I will be offline until Tuesday, September 3, 2024, and will not actively watch over this GitHub during that period.
If anything urgent is needed here, please ask for help in the #expensify-open-source Slack Room-- thanks! If we can wait, I can work on the payment process next week.

I don't have a computer. My laptop is at Apple Service. Sorry

I don't have a computer. My laptop is at Apple Service. Sorry

Confirmed tested on iOS staging! We should be good (finally) for payment! @alexpensify is out until 9/3, but ready when you're back!

@dangrous, thanks for the update! Tomorrow, I'll start working on the payment process.

Payouts due: 4 September 2024

[ ] Contributor: $1000 @fabOnReact
[ ] Contributor+: $1000 @eVoloshchak - please submit a request via Chat, thanks!

Upwork job is here.

@fabOnReact in Upwork says this project was paid in August 2023. Can you please double-check and reply if Upworks's information is inaccurate? Thanks! For now, I'm going to close this GH.

$1,000 approved for @eVoloshchak


You need to specify either text or text_target

/kind bug
What steps did you take and what happened:
[A clear and concise description of what the bug is.]
I try the README.md in the huggingface server https://github.com/kserve/kserve/blob/master/python/huggingfaceserver/README.md
root@63e5f92ee6b6:~/study# kubectl get isvc
NAME               URL                                           READY   PREV   LATEST   PREVROLLEDOUTREVISION   LATESTREADYREVISION                AGE
huggingface-bert   http://huggingface-bert.default.example.com   True           100                              huggingface-bert-predictor-00001   4m55s

Then I run
curl -H "Host: huggingface-bert.default.example.com" xxx.us-west-2.elb.amazonaws.com -d '{"instances": ["The capital of france is [MASK]."] }'

The response is
{"error":"ValueError : You need to specify either `text` or `text_target`."}

What did you expect to happen:
It should give out the result as in the README.md
{"predictions":["paris"]}

What's the InferenceService yaml:
[To help us debug please run kubectl get isvc $name -n $namespace -oyaml and paste the output]
Anything else you would like to add:
[Miscellaneous information that will assist in solving the issue.]
The pod log
2024-03-13 03:19:49.874 uvicorn.access INFO:     <IP_ADDRESS>:0 1 - "POST /v1/models/bert%3Apredict HTTP/1.1" 500 Internal Server Error
2024-03-13 03:19:49.874 uvicorn.error ERROR:    Exception in ASGI application
Traceback (most recent call last):
  File "/prod_venv/lib/python3.10/site-packages/uvicorn/protocols/http/httptools_impl.py", line 419, in run_asgi
    result = await app(  # type: ignore[func-returns-value]
  File "/prod_venv/lib/python3.10/site-packages/uvicorn/middleware/proxy_headers.py", line 78, in __call__
    return await self.app(scope, receive, send)
  File "/prod_venv/lib/python3.10/site-packages/fastapi/applications.py", line 276, in __call__
    await super().__call__(scope, receive, send)
  File "/prod_venv/lib/python3.10/site-packages/starlette/applications.py", line 122, in __call__
    await self.middleware_stack(scope, receive, send)
  File "/prod_venv/lib/python3.10/site-packages/starlette/middleware/errors.py", line 184, in __call__
    raise exc
  File "/prod_venv/lib/python3.10/site-packages/starlette/middleware/errors.py", line 162, in __call__
    await self.app(scope, receive, _send)
  File "/prod_venv/lib/python3.10/site-packages/timing_asgi/middleware.py", line 70, in __call__
    await self.app(scope, receive, send_wrapper)
  File "/prod_venv/lib/python3.10/site-packages/starlette/middleware/exceptions.py", line 79, in __call__
    raise exc
  File "/prod_venv/lib/python3.10/site-packages/starlette/middleware/exceptions.py", line 68, in __call__
    await self.app(scope, receive, sender)
  File "/prod_venv/lib/python3.10/site-packages/fastapi/middleware/asyncexitstack.py", line 21, in __call__
    raise e
  File "/prod_venv/lib/python3.10/site-packages/fastapi/middleware/asyncexitstack.py", line 18, in __call__
    await self.app(scope, receive, send)
  File "/prod_venv/lib/python3.10/site-packages/starlette/routing.py", line 718, in __call__
    await route.handle(scope, receive, send)
  File "/prod_venv/lib/python3.10/site-packages/starlette/routing.py", line 276, in handle
    await self.app(scope, receive, send)
  File "/prod_venv/lib/python3.10/site-packages/starlette/routing.py", line 66, in app
    response = await func(request)
  File "/prod_venv/lib/python3.10/site-packages/fastapi/routing.py", line 237, in app
    raw_response = await run_endpoint_function(
  File "/prod_venv/lib/python3.10/site-packages/fastapi/routing.py", line 163, in run_endpoint_function
    return await dependant.call(**values)
  File "/kserve/kserve/protocol/rest/v1_endpoints.py", line 76, in predict
    response, response_headers = await self.dataplane.infer(model_name=model_name,
  File "/kserve/kserve/protocol/dataplane.py", line 324, in infer
    response = await model(request, headers=headers)
  File "/kserve/kserve/model.py", line 132, in __call__
    else self.preprocess(body, headers)
  File "/huggingfaceserver/huggingfaceserver/model.py", line 170, in preprocess
    inputs = self.tokenizer(
  File "/prod_venv/lib/python3.10/site-packages/transformers/tokenization_utils_base.py", line 2797, in __call__
    raise ValueError("You need to specify either `text` or `text_target`.")
ValueError: You need to specify either `text` or `text_target`.

Environment:

Istio Version: install through https://knative.dev/docs/install/yaml-install/serving/install-serving-with-yaml/#install-a-networking-layer
Knative Version: v1.13.1
KServe Version: v0.12.0
Kubeflow version: none
Cloud Environment:[k8s_istio/istio_dex/gcp_basic_auth/gcp_iap/aws/aws_cognito/ibm]: aws elastic kubernetes service
Kubernetes version: (use kubectl version): client version: v1.25.0, Server version: 1.29
OS (e.g. from /etc/os-release):

It may be I lack the header "Content-Type": "application/json"


Add mouse up down event support

Replacement pr for #289

It's a pity that this hasn't been integrated yet.

please merge this...

Yes, please merge this, as I also need this. Fastclick breaks some other components, i.e. typeahead and select2 when active without that.

Please merge this, i also need this for our tracking library which relies on "mousedown".

Please merge this

FastClick also breaks the Button component of react-toolbox, which also relies on onMouseUp events.
+1 to merge this PR. Thanks!

Please merge yesterday.

This fixed my issue with fastclick and select2, would like to see this merged in too.

Please merge this, it fixed fastclick for my react-quill / quilljs rich text editor.

@rowanbeentje Are we still aiming to support BlackBerry OS2?


Missing code for publishing functions to Azure Function App

In this link: https://github.com/Azure/azure-functions-python-worker/wiki/Create-Function-(CLI), available in the README.me file, the title says "Build and publish using the Functions Core Tools", but in the link for the WIKI you only have the code up to running the function locally and not publishing it back to Azure.
I think the explanation needs to be added after the local testing and the code you should add is the following:
func azure functionapp publish
or you can change the title to only "Build and local testing using the Functions Core Tools".

Thank you for filing an issue about this. The instructions were pulled out temporarily since we discovered a regression in the tools. The wiki is now updated -

https://github.com/Azure/azure-functions-python-worker/wiki/Create-your-first-Python-function
https://github.com/Azure/azure-functions-python-worker/wiki/Developer-Guide


Adding Ember-Shepherd to docs

What's up guys? What would you think about adding Ember-Shepherd officially to the README or docs to link people to using the addon I created?
https://github.com/shipshapecode/ember-shepherd

@b-ash could we make this happen? I'd like to really make Ember-Shepherd top notch and endorsed by you guys.

Hey @rwwagner90, would something similar to this on our tether docs be okay?

@geekjuice yeah, that would be fine. Wherever you guys would normally put links like this. I see you have ember-tether on there, so ember-shepherd being on the Shepherd docs in the same capacity would be great!

Link to reference.

Awesome. Thanks @geekjuice!


Organize game/gallery tags

This is a large PR that may be difficult to review in detail, but it remaps all game tags to a smaller consistent set.  This smaller set was looked at by myself and @recursiveforte, and while it might not be perfect it is at least a little bit more organized.

tutorial
puzzle
strategy
endless
multiplayer
action
sandbox
adventure
memory
timed
music
role-playing
turned-based
real-time
exploration
survival
simulation
utility
sports
retro
platformer
humor
3d

Note - I have noticed in brief review myself that a few titles are mangled (an artifact of a bug in the tag conversion script I wrote for this) - specifically ones that have a colon in the title.  I will repair these manually before landing this PR, they are as follows:

3072: A 2048 Spin-Off
Monkey quest 3: lightless knights
Celeste: Sprig Edition
Coding Demo 3: Custom Directional Solids
Coding Demo 5: Gravity
Ghosbly: Turn Based Combat
SOUL STEALER: The Vengful Spirit
jailbreak :)


HV-1060 Adding Brazilian Portuguese messages for @EAN and @ParameterS…

…criptAssert
@gastaldi Does this look right to you? You are my reference here :) Thx!

Yes, that looks good :)


Modal terminal line-drawing characters ignored

start=$'\e(0'
end=$'\e(B\n'
line='qqqqqqqqqqqqqqqqqq'
echo -en "${start}${line}${end}"

should print (not actually hyphen characters, though -- but should be 18 character widths wide, as many as there are q's):
------------------

actually prints:
qqqqqqqqqqqqqqqqqq

This is actually well understood, and I think the fix should be coming Soontm

FWIW these work with TERM=interix.
I patched my terminfo to get 8 of the colors returned.

Fix is bubbling up through the build system

100%


Latest commits around fear_and_greed

...seem to have broken the code base.
https://github.com/rhettre/coinbase-advancedtrade-python/commit/05079de83c1b3f6b70a85b4af0c7e7dcce475a59#diff-17e1b3aa0cc6edac694fda08db7b4bdb2b9ce45b68587af5287966d487d6bc45
from fear_and_greed import FearAndGreedIndex

May i suggest having one main branch with stable tested code, a development branch used for testing releases on a schedule of 2 wk, 2 months, etc and merging in specific tickets based on issues that need fixed.
Merging straight into main is bad.

What is broken? I've been able to run all the fear and greed functions and limit buys / sells correctly. Did you install the new requirements?

nm, somehow the auto merge dropped off the added module to requirements.txt


[1.1.1] HTTP Error: HTTP GET error on '/?encoding-type=url&list-type=2&prefix=' (HTTP 400)

Hey, sorry just at a bit of a loss with this one. Opening here as I can't reproduce with the npm version of duckdb-node. Hoping you could point me in the right direction - my brain is melting

Anyway, updating to the 1.1.X version of this layer (x86_64 & arm64) I see a 400 error when attempting to read from an r2 bucket. This error does not occur on 1.0.0 of the layer.
I can also not reproduce the issue in the cli locally, or running via duckdb-node locally (both macOS arm64)

To reproduce, it's essentially this:
SET home_directory='/tmp';
SET temp_directory='/tmp/.tmp';
SET custom_extension_repository = 'http://extensions.quacking.cloud';
INSTALL httpfs;
LOAD httpfs;
SET enable_http_metadata_cache=true;
SET enable_object_cache=true;

CREATE SECRET "r2_${bucketName}" (
    TYPE R2,
    ACCOUNT_ID '${accountId}',
    KEY_ID '${keyId}',
    REGION 'auto',
    SECRET '${secret}',
    SCOPE 'r2://${bucketName}'
);

SELECT * FROM read_json(
    'r2://${bucketName}/**/*.json.gz',
    compression = 'gzip',
    columns = {},
    format = 'newline_delimited',
    hive_partitioning = true
);

{
    "errorType": "HTTP",
    "errorMessage": "HTTP Error: HTTP GET error on '/${bucketName}/?encoding-type=url&list-type=2&prefix=' (HTTP 400)",
    "code": "DUCKDB_NODEJS_ERROR",
    "errno": -1,
    "stack": [
        "Error: HTTP Error: HTTP GET error on '/${bucketName}/?encoding-type=url&list-type=2&prefix=' (HTTP 400)"
    ]
}

Notes:

The scoped secret is being used, changing config changes the error
I've tried using a TYPE 'S3' secret and hardcoding the endpoint + switching to s3://*
I've tried loading the httpfs extension from http://extensions.quacking.cloud/ (& the default)
I've tried to diff the source, but my cpp is ~ toddler level

To me it seems like the JS string template values aren’t replaced. In the code you‘re showing single hyphens… Can you check?

@tobilg sorry I cut it down for the example and made it confusing!
I do interpolate for the secret (see https://github.com/duckdb/duckdb/issues/13459), but it does all work locally & look correct if printed
Currently testing if lambda setting AWS_SESSION_TOKEN env vars etc could have any affect (see https://github.com/duckdb/duckdb/blob/8aca4330ac46be3950c6b12e29040322dd245b7a/extension/httpfs/include/s3fs.hpp#L37)

Yep, that did it. Finally!
Some issue with inheritance of the env vars, this fixes it:
process.env.AWS_REGION = '';
process.env.AWS_DEFAULT_REGION = '';
process.env.AWS_ACCESS_KEY_ID = '';
process.env.AWS_SECRET_ACCESS_KEY = '';
process.env.AWS_SESSION_TOKEN = '';

I'll close and open an issue on the main repo

Yeah, seems as though the lambda environment vars being set changes how my R2 secret behaves. Trying to cut it down for a bug report :)

Those variables are always set via the Lambda runtime, see
https://docs.aws.amazon.com/lambda/latest/dg/configuration-envvars.html#configuration-envvars-runtime

Yeah, the new secret cascade feature in 1.1.0 means that the runtime AWS_SESSION_TOKEN gets passed through to the R2 calls, and that causes the 400. Opened this https://github.com/duckdb/duckdb/issues/14201, thanks for your help!
This seems to work:

CREATE SECRET "r2_${bucketName}" (
    TYPE R2,
    ACCOUNT_ID '${accountId}',
    KEY_ID '${keyId}',
    REGION 'auto',
    SECRET '${secret}',
    SCOPE 'r2://${bucketName}',
+   SESSION_TOKEN '',
);


Migrating Nashorn AbstractJSObject

I am migrating legacy code of Nashorn to GraalVM.
There I have a custom array of type  AbstractJSObject.

public final class CustomArray extends AbstractJSObject {

    private final Map<String, Map<String, Serializable>> records;

    private ReferenceArray(final Map<String, Map<String, Serializable>> records) {
        this.records = Collections.unmodifiableMap(records);

    }

    @Override
    public boolean hasSlot(final int index) {
        //some logic
    }

    @Override
    public Object getSlot(final int index) {
        //some logic
    }

    @Override
    public boolean hasMember(final String name) {
         //some logic
    }

    @Override
    public Set<String> keySet() {
        //some logic
    }

    @Override
    public Collection<Object> values() {
         //some logic
    }

    @Override
    public Object getMember(final String name) {
         //some logic
    }

    @Override
    public boolean isArray() {
        return true;
    }
}

how to migrate it?

Have you tried to implement ProxyArray and/or ProxyObject?

Interesting, didn't see them.
will try, hope it gives me everything from above.
will update, thanks.

couldn't find alternative for

  @Override
    public Set<String> keySet() {
        //some logic
    }

    @Override
    public Collection<Object> values() {
         //some logic
    }

couldn't find alternative for ... keySet()

I am not familiar with the exact semantics of AbstractJSObject methods, but isn't ProxyObject.getMemberKeys() what you are looking for?

values()

This seems redundant. It can be derived from keySet() and getMember(), no?


Pick most important terms at build time so ship less data to browser

Working on adding search to my journal which has some ~2000 posts. Search performance is great but I am a bit concerned about how much data I have to ship to the browser. I'm already shipping just an excerpt of each post but even with that the JSON file ends up around ~600kb gzipped.
One idea would be to just ship the title plus short excerpt of each post to the browser along with some precomputed tokens for each post. So then each document would look like { id, title, body, tokens } where tokens would be a string with each token separated by a space.
Does that seem reasonable? If so, is it easy to with your implementation to feed a document in and get out the top say 15 tokens? If not, this package seems to have something for this https://www.npmjs.com/package/natural.

Correct me if I'm misunderstanding you, Kyle- but if you're already doing the work of splitting out the tokens bit, couldn't you just build the local search index using tokens only? (or perhaps tokens plus title, since you'll be sending the titles anyway?)

If so, is it easy to with your implementation to feed a document in and get out the top say 15 tokens?

No. This sounds similar to my tf-idf index, but that wasn't created to consume data and spit out a ranking of tokens. Rather to rank search results by their tokens.

but if you're already doing the work of splitting out the tokens bit, couldn't you just build the local search index using tokens only?

Yeah totally, I just wanted the excerpt for building a nice search results page :-)
Cool! Will try this then.

Cool!
Assuming you split the "interesting" tokens out server-side- which you should, to avoid needing to send the full data down for the search lib to process- you should be able to use:
const { Search } = require('js-search');

const search = new Search('id');
search.addIndex('title');
search.addIndex('tokens');


Syntax error in displaying the HTTPApplicationRoutingZoneName

[Enter feedback here]
The following command doesn't return the required output.
$ az aks show --resource-group myResourceGroup --name myAKSCluster
--query addonProfiles.httpapplicationrouting.config.HTTPApplicationRoutingZoneName -o table
The right syntax is
az aks show --resource-group myResourceGroup --name myAKSCluster \
--query addonProfiles.httpapplicationrouting.config.HTTPApplicationRoutingZoneName -o table
Result
39879fed64034850ab8a.westeurope.aksapp.io
This can be evident from
az aks show --resource-group myResourceGroup --name myAKSCluster
"addonProfiles": {
"httpapplicationrouting": {
"config": {
"HTTPApplicationRoutingZoneName": "39879fed64034850ab8a.westeurope.aksapp.io"
},
"enabled": true
}
},

Document Details
⚠ Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

ID: 51a32f9e-c92f-412b-d460-567f3c542b98
Version Independent ID: fad073ab-a376-1535-7bc4-deaca1d98dd9
Content: HTTP application routing add-on on Azure Kubernetes Service (AKS)
Content Source: articles/aks/http-application-routing.md
Service: container-service
GitHub Login: @lachie83
Microsoft Alias: laevenso

@kumarallamraju Thanks for the feedback! We are currently investigating and will update you shortly.

@kumarallamraju PR has been raised to fix the code snippet issue, changes will go live once it is approved.

@kumarallamraju camelCase 'HttpApplicationRouting' should be all lower case 'httpapplicationrouting'
Thanks for pointing that out!

I am proceeding to close the issue, the document changes will reflect once the PR is approved.
Thanks.


Footer is displayed on the top of the page and overlaps on the logo in contributor console

Background

Launch the github repository (which is configured to sign ICLA)
Login as contributor
Update any file and create PR
Checks fail
Click on the link to sign CLA
Contributor is redirected to contributor console
Click on Individual option
Observed that footer is displayed on the top of the page and overlaps on logo

@amolsontakke3576 please review the layout/footer to see if we can minimize the footer size and its ability to take over the UI....

Verified in FF/Chrome/Edge and observed that the issue is fixed


datepicker 2 migration 27 added, default response udpated

Description

add iso date formate for default min and max date and add migration script for change default min and max date for already exists component

Fixes #4945
Type of change

Bugfix (non-breaking change which fixes an issue)

Checklist:

[x] My code follows the style guidelines of this project
[x] I have performed a self-review of my own code
[ ]  I have commented my code, particularly in hard-to-understand areas
[ ]  I have made corresponding changes to the documentation
[x] My changes generate no new warnings
[ ]  I have added tests that prove my fix is effective or that my feature works
[x] New and existing unit tests pass locally with my changes

Test coverage results :test_tube:
:red_circle: Total coverage has decreased
// Code coverage diff between base branch:release and head branch: bug/date-picker-iso-migration

Status
File
% Stmts
% Branch
% Funcs
% Lines

:red_circle:
total
53.81 (-0.02)
35.78 (-0.02)
32.41 (-0.03)
54.39 (-0.02)

:red_circle:
app/client/src/utils/WidgetPropsUtils.tsx
63.87 (-1.43)
57.32 (-1.32)
53.49 (-2.46)
63.1 (-1.48)

/ok-to-test sha=26c654c

@techbhavin Please rebuild the deploy preview after taking latest pull from release branch. Currently we are not able to open applications on safari due to issue #5480

@somangshu Verified this and working as expected


DNS blocklists: Disabling an existing list filter with adguard_list_filter

Hello.  Thank you very much for your work!
I have a fresh AdGuard Home installation and I am using Terraform to configure it.  Ideally I would avoid using the user interface for any modifications.
My goal is to disable an existing DNS blocklists but I Terraform returns an error "Filter with URL "https://adguardteam.github.io/HostlistsRegistry/assets/filter_1.txt": url already exists".

resource "adguard_list_filter" "AdGuard_DNS_filter" {
  name      = "AdGuard DNS filter"
  url       = "https://adguardteam.github.io/HostlistsRegistry/assets/filter_1.txt"
  enabled   = false
  whitelist = false
}

May I know if what I am trying to achieve is possible?
Thanks!

If you need to manage an existing resource that was created outside of Terraform, you need to import it first to your state.
https://registry.terraform.io/providers/gmichels/adguard/latest/docs/resources/list_filter#import
After importing, you will be able to manage it as you wish.

Indeed, the import worked.
terraform import adguard_list_filter.AdGuard_DNS_filter "1"
Apply

I am adding an import step to my workflow.  Thanks!


[kjobctl] Embed the CRDs in it's binary

What type of PR is this?
/kind feature

What this PR does / why we need it:
An admin can use the output of kjobctl crds to install, update or remove them in/from a cluster without other dependencies.
  # Install or update the kjobctl CRDs
  kjobctl crds | kubectl apply --server-side -f -
  # Remove the kjobctl CRDs
  kjobctl crds | kubectl delete --ignore-not-found=true -f -

Which issue(s) this PR fixes:

Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?

None

/cc @mwielgus @mbobrovskyi

/assign @mwielgus

/lgtm


liveness probe and readiness probe of ks-jenkins failed

OS info:
CentOS Linux release 7.9.2009 (Core) , 4C/16G
Kubernetes version:

Docker version:

KubeSphere version:
v3.1.1
Issue Description:
After I finish KS installation offline, I enable DevOps by following the step of Enable DevOps after Installation
Here list the snapshots:
clusterconfiguration

ks-jenkins pod status and events:

log of ks-jenkins pod

Helm list

I find that jenkin uc is not deployed, is this the reason why ks-jenkins is not ready
Please, experts  come up with some solutions or ideas!

/kind support
Thanks for reaching out here. There are several step you can try:

Describe the Pod ks-jenkins and check the exit code of Container ks-jenkins
Check latest events
Check full logs of previous Pod ks-jenkins

I find that jenkin uc is not deployed, is this the reason why ks-jenkins is not ready

All Jenkins plugins are installed in the ks-jenkins container image since v3.x. So, we don't need a private uc (Jenkins update center) any more.

/kind support Thanks for reaching out here. There are several step you can try:

Describe the Pod ks-jenkins and check the exit code of Container ks-jenkins

Check latest events

Check full logs of previous Pod ks-jenkins

@JohnNiang, I did some checks as you said, but still could not get obvious error info. Here screenshots listed above is what I do,  please provide some in-deep analysis or directions

I find that jenkin uc is not deployed, is this the reason why ks-jenkins is not ready

All Jenkins plugins are installed in the ks-jenkins container image since v3.x. So, we don't need a private uc (Jenkins update center) anymore.

Ok, thanks for quick reply!

Try remove liveness probe and readiness probe, then see if it can start successfully.

Try remove liveness probe and readiness probe, then see if it can start successfully.

It's working now after removing liveness probe and readiness probe. However, I am confused that http://<POD_IP>:8080/login  is only used by probes to do health check or also plays other role in jenkins.
If the latter, even though ks-jenkins is ready, http://<POD_IP>:8080/login still refuses connection, will there be potential risk?

I guess your cluster might have a network issue.

I guess your cluster might have a network issue.

It's wired, in my cluster, every component is running well, I run curl -v http://<IP_ADDRESS>:8080/login in my cluster and got response 200.

Anyhow, it is runing now, if any new issues I found, I will be back, and ask for support.
@LinuxSuRen, thanks!

The same problem was assumed to be caused by the NTP server, but I tried to set jenkinsJavaOpts_Xms and jenkinsJavaOpts_Xmx to less than 1000M, and the problem was resolved。


improving getting started instructions

I have almost no frontend experience, but I wanted to try outwatch to make a basic UI.
It would be really nice to have the most basic possible hello world example with no skipped steps.
I have an existing project, so I wanted to make the minimal addition to my sbt build, but I didn't have problems there. The first problem I had was:

When starting the hello world example from here: https://outwatch.github.io/docs/readme.html I didn't see what the exact path to the .js file. I should use. Eventually, by looking at scalajs, I found that I should run fastLinkJS and then use the file that was printed, but that didn't work. The snabbdom library seems to be using a require function that doesn't work on browsers.
I saw that I needed to add the bundler plugin but I didn't see that I had to do anything extra. The next thing I had to figure out was that I need to run fastOptJS::webpack to solve this issue.
But on my M1 mac, brew has installed for me Node 21.6.1 today. So the next issue I hit was this: error:0308010C:digital envelope routines::unsupported around a Hash instantiation. In order to solve that, I exited sbt and at the shell did export NODE_OPTIONS=--openssl-legacy-provider and then restarted sbt. Then the build command worked.

I know it's a challenge to make a tutorial accessible to someone with as little knowledge as myself on the subject, but I spent about 2 hours setting up and solving the above issues, when it seems like we could have a set by step with no missing bit (e.g. the readme doesn't say what path to JS to put, or how to find it, nor does it tell you which sbt task to use).
Anyway, maybe this issue can be of help to other similiarly clueless folks as myself.

First of all thank you very much for opening this issue and writing down the points of struggle!
It is definitely not super easy to set it up and there a few gotchas. I am on holiday right now, but will try to address these points when i come back next week.
There is also the question of using webpack (i.e. ScalaJSBundler) or vite for bundeling your javascript code and it dependencies. Currently all our tutorials point to the former - and i want to update that as well.
Please bear with me and let me know about any other issue you encounter.

For the record, we now have an example using vite here: https://github.com/outwatch/example-mill-vite. The documentation section on the website mentions the new example. But I think, there are still a few more points to mention about the setup. Keeping this open for now.


Don't force the consumers to pull in unnecessary deps

Coverage remained the same at 95.484% when pulling ceced2c4de6a12139b37348250adfb4a87f43eeb on optional-deps into 7fb23e7b9882e0b2bf94fa3a1204810c90c2ccc4 on master.


Tooltips for Greek letters

Hi,
I'd like to suggest a small improvement. It would be nice if Greek letters were described using their names like "rho" when highlighted. Currently, the tooltip only says "Greek letters":

Also, it shows the same description when highlighting e.g. the ‰ or ° symbol.

Thank you!
This tooltip is assigned to the whole panel. That is why it shows the same for all items inside.
Currently, we are still working on making Calcpad  localizable so, if there are any changes to the UI, we'd better do it after that.

Great, thanks for the information. I'll be glad to help with Polish localization btw.


Add section on phased rollouts for dynamic groups

Changelog: Title
Ticket: None
Signed-off-by: Alan Martinovic<EMAIL_ADDRESS>

@lluiscampos Added you with hopes you can help me understand whats causing the test to fail.

You are submitting this PR from a branch that it is many months old :sweat_smile:

If you rebase on top of mendersoftware/master the failing tests should pass.
Note that your commit is not following the modern commit syntax so that CI check will fail after the rebase.

@lluiscampos Thanks, my bad :)


Remove submodules

We should remove submodules, similar to what we did for SwinjectStoryboard here: https://github.com/Swinject/SwinjectStoryboard/pull/44
Submodules prevent Carthage from sharing Swinject when using multiple Swinject dependencies, and can lead to build errors when including Carthage dependency projects as sub-projects.

Thanks a lot, merged in https://github.com/Swinject/SwinjectAutoregistration/pull/41


SCRIPT5022: Exception thrown and not caught

HTML page:
<!DOCTYPE html>
<html>
	<head>
		<title>JSON3 Test</title>

		<!-- JSON3 library (Development)
		<script type="text/javascript" src="/e2/js/json3_v3.3.2.js"></script>

        <!-- JSON3 library (Minified) (Production)
        <!-- Error occurs with minified file, as well -->
		<!--<script type="text/javascript" src="/e2/js/json3_v3.3.2.min.js"></script>-->

		<script type="text/javascript">

            // Purposely incorrect to cause exception
			// Error: Reporting thrown exception not caught
			JSON.parse( "[1, 2, 3)" );

		</script type="text/javascript">

	</head>

	<body></body>
</html>

As you can see from the above sample HTML page, I purposely used an incorrect string to throw an exception. The exception is not getting caught. In fact, the error it produces the following in Microsoft IE7 through IE11:

SCRIPT5022: Exception thrown and not caught
File: json3_v3.3.2.js, Line: 568, Column: 11

In Google Chrome, the following error appears and can be visible with the developer toolbar:

Uncaught SyntaxError: Unexpected token )

Now, my question is this: Is this by design or a bug? I can easily fix this by catching the exception, i.e. wrap the parse method with a try..catch block. But if I'm using a third-party plugin which relies on JSON parsing and it expects the JSON3 library to catch the exception, I may have to wait until the author fixes his/her plugin.
Another workaround, particularly for legacy versions of IE, is to employ the JSON2 library (https://github.com/douglascrockford/JSON-js). I did not experience any errors related to uncaught thrown exceptions with the JSON2 library. I didn't look at the code, but I gather it takes care of catching these exceptions.
So, again, is this uncaught exception by design and it's up to the developer to catch it or is it a bug and JSON3 should take care of catching exceptions?
Thank you very much.

The exception is thrown due to invalid JSON syntax: [1, 2, 3). Since you did not wrap your JSON.parse usage in a try-catch block, the exception is not caught and bubbles up, which is the expected behaviour, as per the spec. If JSON 2 does not do so, it is an error on their part.


'Meteor' object has no attribute 'lock'

When I did my 'Meteor 'evaluation, an error occurred.

Excuse me, have you solved this problem? I was  in the same situation.

Hello, I have the same problem.
I found that this problem may be related to the fact that Java is not installed on the system.
You can test whether Java is installed in the system through command java -version.
If Java is not installed, you can execute command sudo apt update sudo apt install openjdk-11-jdk.
I hope it works for you!

Thank you so much for answer! @yvfengZhong

Hello, I have the same problem. I found that this problem may be related to the fact that Java is not installed on the system. You can test whether Java is installed in the system through command java -version. If Java is not installed, you can execute command sudo apt update sudo apt install openjdk-11-jdk. I hope it works for you!

Thanks for your answer