Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[CodeEditor] Potential bugs due to stale onChange callback #175684

Closed
Dosant opened this issue Jan 26, 2024 · 1 comment · Fixed by #176296
Closed

[CodeEditor] Potential bugs due to stale onChange callback #175684

Dosant opened this issue Jan 26, 2024 · 1 comment · Fixed by #176296
Assignees
@Dosant @eokoneyo
Labels
bug Fixes for quality problems that affect the customer experience Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience)

Comments

@Dosant
Copy link
Contributor

Dosant commented Jan 26, 2024

Kibana version: main

Describe the bug:

After an upgrade of dependencies code editor dependencies #171720 we noticed an issue with onChange callback where it always has the stale closure (only initial values)

You can see the reproduction in the story in this PR #175670
This is likely the same problem that is reported here react-monaco-editor/react-monaco-editor#704

Potentially, a bunch of user-facing bugs could be caused by this where CodeEditor is used. For example, I found a bug with URL template editor where placeholder is not shown when you empty the editor:

placeholder doesn't work:

placeholder.works.mov

placeholder works:

no.placeholder.mov

Any additional context:

  • Potentially we can fix this by a workaround inside CodeEditor component
  • We might consider patching react-monaco-editor
  • We can also remove react-monaco-editor wrapper and use a custom wrapper
@Dosant Dosant added the bug Fixes for quality problems that affect the customer experience label Jan 26, 2024
@botelastic botelastic bot added the needs-team Issues missing a team label label Jan 26, 2024
@Dosant Dosant added the Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) label Jan 26, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/appex-sharedux (Team:SharedUX)

@botelastic botelastic bot removed the needs-team Issues missing a team label label Jan 26, 2024
@Dosant Dosant mentioned this issue Feb 6, 2024
Merged
Dosant added a commit that referenced this issue Feb 7, 2024
@Dosant
[CodeEditor] Explicitly call the latest props.onChange (#176296)
4f3bc35 
## Summary

fix #175684

Refer to the issue #175684 for
the problem and reproduction


I tested that this fixes the issue using the story from the reproduction
#175670 and the URL drilldown's
placeholder issue explained in
#175684
@Dosant Dosant self-assigned this Feb 7, 2024
fkanout pushed a commit to fkanout/kibana that referenced this issue Feb 7, 2024
@Dosant @fkanout
[CodeEditor] Explicitly call the latest props.onChange (elastic#176296
51d1d12 
)

## Summary

fix elastic#175684

Refer to the issue elastic#175684 for
the problem and reproduction


I tested that this fixes the issue using the story from the reproduction
elastic#175670 and the URL drilldown's
placeholder issue explained in
elastic#175684
CoenWarmer pushed a commit to CoenWarmer/kibana that referenced this issue Feb 15, 2024
@Dosant @CoenWarmer
[CodeEditor] Explicitly call the latest props.onChange (elastic#176296
06701a4 
)

## Summary

fix elastic#175684

Refer to the issue elastic#175684 for
the problem and reproduction


I tested that this fixes the issue using the story from the reproduction
elastic#175670 and the URL drilldown's
placeholder issue explained in
elastic#175684
fkanout pushed a commit to fkanout/kibana that referenced this issue Mar 4, 2024
@Dosant @fkanout
[CodeEditor] Explicitly call the latest props.onChange (elastic#176296
379d117 
)

## Summary

fix elastic#175684

Refer to the issue elastic#175684 for
the problem and reproduction


I tested that this fixes the issue using the story from the reproduction
elastic#175670 and the URL drilldown's
placeholder issue explained in
elastic#175684
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Dosant Dosant

@eokoneyo eokoneyo

Labels
bug Fixes for quality problems that affect the customer experience Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[CodeEditor] Explicitly call the latest props.onChange Dosant/kibana
3 participants
@Dosant @eokoneyo @elasticmachine