-
-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement custom events suggestions #4092
Conversation
6f8f166
to
ad215cf
Compare
ad215cf
to
379d02a
Compare
0c4517b
to
834ba1f
Compare
7ccfec9
to
a6147a2
Compare
a6147a2
to
4592f96
Compare
4592f96
to
f34ceb7
Compare
ce3ee10
to
50e0b77
Compare
This reverts commit 1b1c801.
f20d6ad
to
ccd0265
Compare
ccd0265
to
691830b
Compare
So component state hacks aside, I think there's still some room for improvement in the auto-completion dialog - see attached screen recordings.
record-2024-06-10-11-01-28-vier.saul.pews.mp4record-2024-06-10-11-15-04-debt.vase.faro.mp4 |
That might work for native stats but imported custom events do not have hostname associated with them. Besides, that garbage is usually a result of security scans which AFAIK query valid hostnames too? |
Automatic addition feature considers events that appeared in stats within last 6 months - and so do suggestions when adding a name. The problem is - the "automatic addition" query uses the default of first 25 results while rejecting all existing, configured goals from that list. |
b8e126c
to
d355c6c
Compare
d355c6c
to
4aa9579
Compare
The PR is ready for another round of review \cc @aerosol @RobertJoonas |
Great! Almost there: record-2024-06-10-13-58-31-awed.chao.duck.mp4 |
I can see that happening for any combobox input when you select the same suggestion the second time - also in prod 🙈 I'll see if I can do something about it however - maybe in a follow-up PR? |
53f452a
to
5f34cc1
Compare
5f34cc1
to
a041ba8
Compare
This reverts commit a041ba8.
Changes
Preview deployment available at https://pr-4092.review.plausible.io (access possible using staging credentials)
Slow connection can be emulated by executing
liveSocket.enableLatencySim(900)
in browser's JavaScript console.Depends on #4033The primary purpose of this PR is adding support for suggesting event names when defining custom goal events.
The secondary purpose (which turned out to be much more time consuming), is addressing a problem of live components embedded inside a modal perserving their state through close/open cycle when done fast enough. Live components do not have an explicit
unmount
phase in their lifecycle. Instead, they are pruned from LiveView state when they are no longer visible in DOM structure on the frontend, after applying an LV patch. The pruning process is done in 2 stages, with 2 roundtrips to the server and the actual removal from the backend state is done during the second one. If a given components becomes reachable in DOM again between those roundtrips, it is not going to be removed. My understanding is that it's not possible to tell with certianty that disappearance from DOM is fully intended or just the effect of intermediate patch application in rapid succession.Initially, I have tried to find a way to ensure modal is reopened only after all the components inside it are removed from BE state. However, LV does not expose any way to inspect internal components state this way. I could only achieve that via a hack where I've eavesdropped process' state via
:sys.get_state/1
in f34ceb7. Even though the solution worked, it was not acceptable for production use because:sys.get_state/1
shouldn't be used anywhere outside debugging code. Another important factor was longer wait time for components to disappear with periodic probing of state, which was pretty wasteful. I eventually went with that problem over to #phoenix at Elixir Slack (question asked there quoted below).In the end, the only workable solution turned out to be making all embedded live components' IDs unique with every close/open cycle of the modal. That applies to every nested live component, unfortunately, as that "liveness issue" applies to all levels of component hierarchy. The modal generates a unique, predicatable modal ID changed with every close/open cycle. This ID should be appended to all live components' IDs inside the modal - unless state reset is implemented explicitly. The sequentiality of the unique ID makes it predicatable enough for testing purposes. One drawback of this approach is that component's IDs can't be determined outside modal component (so, for instance,
send_update
using ID from outside the modal might not be viable). However, if state is already explicitly manipulated from the outside, managing "reset" state explicitly is usually expected anyway.I have also applied the same principle to fix the same "liveness" problem when switching tabs inside the modal. Every tab switch generates another sequential ID which is appended in top of sequential modal ID.
Quote of the original post on Slack follows:
TODO
Tests
Changelog
Documentation
Dark mode