Open ninoslavmiskovic opened 11 months ago
Pinging @elastic/kibana-visualizations (Team:Visualizations)
This also happens in Lens formula so I suspect that is either an EUI or a monaco issue. We need to investigate who is the culprit here
This also happens in Lens formula so I suspect that is either an EUI or a monaco issue. We need to investigate who is the culprit here
++ . I was also thinking maybe to have a UX proposal that can "unblock" us. Perhaps @MichaelMarcialis has an idea we can work with :)
A possible alternative would be #166089 (as it happens in Lens formula) rather than having the popover open.
It sounds like two different issues are being discuss here, and I want to make sure I'm understanding. Please let me know if the following is true:
Is my understanding correct? If so, issue 1 is definitely a bug and not functioning as intended. @stratoula appears to be correct that it only occurs when attempting to click on the Monaco editor while the popover is open. Let me know if there's anything I can do to help fix the issue.
Regarding issue 2, I like @dej611's suggestion above. A similar thought (which also takes a page from what we do with Lens formula) would be to change the behavior of the references when the editor is in expanded mode. Rather than displaying a popover when the button is clicked, we can use it to show/hide a sidebar with the reference documentation. Doing so would allow users to edit their query while keeping the reference opened simultaneously.
Alternatively, if we feel the ability to edit and view the documentation in the same view should be always available (regardless if the user is in expanded edit mode or not), we can always explore additional possibilities. It could be as simple as losing the popover altogether and making it a toggleable panel regardless of compact/expanded mode. Or maybe creating a new popover capability that allows users to optionally pin popover contents above all other page contents and support moving/resizing as needed.
In any case, let me know how we'd like to proceed and prioritize this, and I'd be happy to support on the design side.
I think we just need to fix the bug meaning that when a user is clicking the editor to close the popover. I agree with Michael that authoring and having the documentation visible at the same time is a different thing. @ninoslavmiskovic wdyt?
hi all,
I couple of comments.
I agree that there is both a small bug 1) and a feature request on the ability to edit and view documentation at the same time). We can create a separate issue for the bug, so the user can exit the popover when clicking on the editor.
This is an enhancement around 2) view the ES|QL reference and author your query at the same time, without having to constantly reopen the reference popover.
@dej611 - I like your enhancement and this would help educate users for sure, so we should continue that effort. However, I still see the value for the user to have the ability to see the entire documentation page, to make it easier to e.g. follow a 1,2,3 step guide on a full page.
@MichaelMarcialis
I like your approach to the formula and perhaps we can place it under the query bar (Maybe the height will be a challenge here) or as a push-flyout (this might be challenging because of the amount of text, so width can perhaps be an obstacle, if we use the same fly-out width as "editing a visualization" )
Or perhaps also give the user the ability to "pin" the popover, if they don't want to go into full mode, which allows them to pin the popover or make it sticky and give them the capability to edit the query while having the documentation open to e.g. copy/paste or see the query for reference.
Or maybe creating a new popover capability that allows users to optionally pin popover contents above all other page contents and support moving/resizing as needed.
Or perhaps also give the user the ability to "pin" the popover, if they don't want to go into full mode, which allows them to pin the popover or make it sticky and give them the capability to edit the query while having the documentation open to e.g. copy/paste or see the query for reference.
It sounds like we both have a similar idea here. Personally, I'd love to explore this option, as I think it would be a pattern that could be adopted in a number of areas across Kibana and helps avoid overcomplicating already complex app interface layouts. Is this something we want to prioritize in the short-term for an upcoming release, or more of a long-term goal?
I think we should have a design proposal on how we can solve this before GA for sure . It would help users learn ES|QL 👍
I can plan to do some design exploration around this. How would you like this to be prioritized, @ninoslavmiskovic? Should it come before the design work @andreadelrio and I are doing on ES|QL history? Immediately after? Other?
In parallel 😊 if possible 👍
There is currently a bug when the ES|QL reference popover is open that prevents you from being able to click outside the popover to close it (if the click is on the Monaco editor).
@elastic/eui-team I wonder if you have any idea about this part. To give some context, we have a popover with the ES|QL documentation and the monaco editor for writing ES|QL.
We expect that when a user clicks inside the editor, the click event will fire and will close the popover and expand the editor. It doesnt though (in other places it works as expected, it is only the monaco editor which causes the problem.
I tried to wrap the popover in the EuiOutsideClickDetector
component This will indeed close the popover when I click the editor but I need to click another time for the editor to expand.
Maybe you have any idea on what is going on and what else we could try?
Thank you!
Hey @stratoula, I don't understand what you are asking and I'm not familiar with this code. In terms of EUI components, what is not working as expected?
@JasonStoltz I think is mostly a question if you have any idea of why when clicking inside the monaco editor, the EUIPopover doesnt close.
Is the Monaco editor in an iframe? That would be the main reason I could think of for the outside click to not work as expected. If so, this would also be a possible issue or configuration with our underlying focus trap component/3rd party library, FWIW.
Hmmm, interesting. No the Monaco editor in not in an iframe. We just use the component from the third party plugin afaik. Nothing fancy here.
Gotcha. Based on what you said here:
I tried to wrap the popover in the
EuiOutsideClickDetector
component This will indeed close the popover when I click the editor but I need to click another time for the editor to expand.
My best guess is that the editor/third party plugin has its own click events/detectors that is swallowing or stopping/preventing our popover's focus trap/outside click logic. This has a few possible approaches, depending on your preference for hackiness vs frustration levels:
onClick
(either a div wrapper that prevents the event from capturing within, or an invisible div overlay that sits on top of the editor) that manually performs the state updates you need - closing the popover and toggling the editor (probably faster but also pretty hacky 😬)Thanx Cee! I was thinking the same approaches but none of them is super appealing to me 😄 Maybe the best and cleanest approach is the redesign 🤷♀️
Yeah, that's definitely a lot of information to have in a popover - a redesign would be ideal!
Thanx Cee! I was thinking the same approaches but none of them is super appealing to me 😄 Maybe the best and cleanest approach is the redesign 🤷♀️
@stratoula, @cee-chen: Forgive me if I'm misunderstanding, but I'm not sure the redesign for the inline documentation will fully address the issue. The most recent wireframe designs enhancements I suggested still house the inline documentation in a popover. And while these designs do offer the option to "pin" the panel (preventing it from being closed when shifting focus elsewhere), the initial state is still that of a standard popover. Additionally, I have to imagine this click-outside-not-being-honored issue still adversely affects the closing behavior for all other popovers, not just the inline documentation one.
For example, open the super date picker's popover and try to click on the ES|QL editor. The same problem persists.
@MichaelMarcialis sorry I understood after a discussion I had with Nino that we are going to rethink the implementation and go with something else so I was hoping to take this under consideration.
None of the hacky ideas mentioned above are ideal and we are not even sure that it will solve the problem 🤷♀️
But you are right, time picker is another problem. This definitely needs investigation.
@MichaelMarcialis
@stratoula is correct. Solving this challenge will be part of the longer-term evolution of the in-line documentation experience. That is currently in the backlog as "Discovery" stage idea and we need to move it to "ready-for-development" and prioritize it against the other ideas.
Short term we are going to do the tasks in this issue: https://github.com/elastic/kibana/issues/173495 , which with the link to full documentation will in-directly help the user have a separate tab open while writing the query.
So I did a very small investigation
I used the focusTrapProps property on the EUIPopover to console a message and apparently it logs it everywhere except from the monaco editor app. It doesn't recognize it as an outside area.
focusTrapProps={{
clickOutsideDisables: false,
onClickOutside: () => {
console.log('meow');
},
}}
I solved it!!
Here is my PR https://github.com/elastic/kibana/pull/176394
What I did:
My solution doesn't work for the super duper date picker though because I need to wrap it on the EuiOutsideClickDetector and force it to close but I don't have access to this. Maybe @cee-chen you can help with this? Add a way to programmatically close the super date picker or something like that?
Maybe @cee-chen you can help with this? Add a way to programmatically close the super date picker or something like that?
Unfortunately there isn't a super easy way to do this that I can see. It might be faster/easier to work around this via vanilla JS (i.e. if a popover is open, use a JS .click()
on the the button that toggled it to close it)
EDIT: The other option is I could forward a ref to the internal class component that would allow you to call ref.current.onStartDatePopoverClose()
/onEndDatePopoverClose()
🤔 But that's also definitely more of a workaround territory as well.
Pinging @elastic/kibana-esql (Team:ESQL)
Does @eokoneyo's monaco fix in #178622 resolve this issue?
@cee-chen back to @MichaelMarcialis 's comment:
1) There is currently a bug when the ES|QL reference popover is open that prevents you from being able to click outside the popover to close it (if the click is on the Monaco editor). This disrupts your ability to continue authoring your query (and runs counter to how other Kibana popovers are expected to behave). 2) You'd like to be able to view the ES|QL reference and author your query at the same time, without having to constant reopen the reference popover.
https://github.com/elastic/kibana/pull/178622 resolved the bug (item 1), but I believe this issue was primarily intended to describe the enhancement request of viewing the reference while editing the query (item 2). It just so happens that the video includes the bug as well so we started off on that discussion (cc @ninoslavmiskovic correct me if I'm wrong).
So I would say that EUI's involvement here can be concluded. Thank you for your help on resolving the bug!
The request of viewing documentation concurrently with query editing remains valid in my view. I will update the video in the description so as to avoid future confusion about the bug!
Correct! I solved the bug here https://github.com/elastic/kibana/issues/166907#issuecomment-1931961873 but I didn't close the issue as it was about the enhancement request.
@MichaelMarcialis @ninoslavmiskovic @stratoula can we re-scope this issue. Now that the bug is solved I think this issue should just be a design that anchors the documentation to the right when the textarea is expanded. This solved the problem of leaving the documentation open for reference while editing a query.
I can do that for sure - @timductive
@MichaelMarcialis @ninoslavmiskovic @stratoula can we re-scope this issue. Now that the bug is solved I think this issue should just be a design that anchors the documentation to the right when the textarea is expanded. This solved the problem of leaving the documentation open for reference while editing a query.
@timductive: By "anchors the documentation to the right", do you mean via push flyout? If so, I'd be concerned about the scalability of such an approach. For example, while that approach might work when the query editor is in the unified search section, it becomes more problematic when the query editor is already in a flyout (such as the inline editor). It can also become problematic when other competing flyouts are in play (i.e. do these competing flyouts cover or replace the documentation?). This is why I was more keen to explore solutions that would be independent from the page layout (either a resizable, repositionable panel or a separate chromeless browser window). Thoughts?
The idea of a resizable panel was recently suggested (i.e. not a flyout). To further the discussion, here is a wireframe depicting such a layout.
As Michael notes, there could be scalability and system concerns depending on the implementation. For the inline editor, this may continue opening in a popover. For the 'full' editing experience, we may want to encourage users to work in Discover (this presumes a trusted dip-in/dip-out seamless flow).
Feature Description:
To improve user experience in ES|QL editor, this issue proposes a design enhancement where the documentation is anchored to the right side of the screen when the query editor textarea is expanded.
This feature will allow users to view and interact with ES|QL documentation seamlessly while editing queries, facilitating a more intuitive and educational user interface.
The anchored documentation should be toggleable and resizable to accommodate different user preferences and screen sizes.
Current problem it is solving:
As a user, I want to be able to have the in-line documentation open while I am typing the query so that I can easily copy/paste or type the right syntax.
The below video shows that I can not continue working on my query while having the documentation open.
https://github.com/elastic/kibana/assets/315764/5ac781af-709f-4696-8ea7-8d97c827f888
Design/wireframes:
Design proposal from @MichaelMarcialis : https://whimsical.com/wireframes-Uv1H1D6Ydt63EKqEJSMB28 <= Needs update where the panel is anchored to the right, when the textarea is expanded.