Skip to content

Commit 921a43d

Browse files
committed
This patch includes updates to multiple Java classes within the com.osiris.desku.ui package. The changes primarily focus on improving the functionality of UI components and enhancing debugging capabilities. Below are the key changes:
1. **FileChooser.java**: - Added a line to toggle the visibility of `directoryView` when a file path button is clicked. 2. **Tooltip.java**: - Simplified the JavaScript execution by directly using `parent.executeJSForced` instead of fetching UI context separately. 3. **UI.java**: - Modified `executeJavaScriptSafely` to return a `CompletableFuture<Void>`, allowing asynchronous handling of JavaScript execution. - Introduced deeper debugging by capturing stack traces and logging detailed component hierarchy information. - Refined the process of safely attaching components to their parents, ensuring that parents are attached before their children. - Added handling for invalid parent scenarios, which can indicate that a component was not properly added to a parent. 4. **Component.java**: - Added an `executeJSForced` method to ensure JavaScript code is executed even if the UI is still loading. - Enhanced event handling for component attachment and detachment, improving the reliability of JavaScript execution tied to component lifecycle events. - Introduced a stack trace capture feature for better debugging when components are created. 5. **FileChooserTest.java**: - Modified the test method to use `testIndefinitely` instead of `testAndAwaitResult`, adapting to the changes in `FileChooser`. These changes improve the robustness, flexibility, and debugging capabilities of the UI components in the Desku project.
1 parent 1e1c67e commit 921a43d

File tree

5 files changed

+162
-61
lines changed

5 files changed

+162
-61
lines changed

src/main/java/com/osiris/desku/ui/Component.java

Lines changed: 40 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,15 @@ public class Component<THIS extends Component<THIS, VALUE>, VALUE> {
6363
* </pre>
6464
*/
6565
public final int id = idCounter.getAndIncrement();
66+
/**
67+
* Always null, unless {@link App#isInDepthDebugging} is enabled. <br>
68+
* Stacktrace to find where in your code this component was created. <br>
69+
*/
70+
public StackTraceElement[] createdAtStracktrace = null;
71+
{
72+
if(App.isInDepthDebugging)
73+
createdAtStracktrace = new Exception().getStackTrace();
74+
}
6675
/**
6776
* List of children. Normally it's read-only. <br>
6877
* Thus do not modfify directly and use methods like {@link #add(Component[])} or {@link #remove(Component[])}
@@ -105,9 +114,9 @@ public class Component<THIS extends Component<THIS, VALUE>, VALUE> {
105114
public final Event<ValueChangeEvent<THIS, VALUE>> readOnlyOnValueChange = new Event<>();
106115
/**
107116
* Gets executed when this component <br>
108-
* was attached to the UI.
117+
* was attached or detached from the UI.
109118
*/
110-
public final Event<Void> _onAttached = new Event<>();
119+
public final Event<Boolean> _onAttached = new Event<>();
111120
public final ConcurrentHashMap<String, String> style = new ConcurrentHashMap<>();
112121
/**
113122
* Gets set to false in {@link AddedChildEvent}. <br>
@@ -121,7 +130,7 @@ public class Component<THIS extends Component<THIS, VALUE>, VALUE> {
121130

122131
public THIS setAttached(boolean b){
123132
isAttached = b;
124-
_onAttached.execute(null);
133+
_onAttached.execute(b);
125134
return _this;
126135
}
127136
public boolean isAttached(){
@@ -149,6 +158,8 @@ public boolean isAttached(){
149158
*/
150159
public MyElement element;
151160
public Consumer<Component> _remove = child -> {
161+
if(App.isInDepthDebugging)
162+
AL.debug(this.getClass(), "Attempting to remove child '"+child.toPrintString()+"' from parent '"+this.toPrintString()+"'.");
152163
UI ui = UI.get(); // Necessary for updating the actual UI via JavaScript
153164
if (children.contains(child)) {
154165
children.remove(child);
@@ -441,14 +452,32 @@ public boolean isValuesEqual(VALUE val1, VALUE val2){
441452
* this component via the "comp" variable in your provided JavaScript code.
442453
*/
443454
public THIS executeJS(String code){
444-
return executeJS(UI.get(), code);
455+
return executeJS(UI.get(), code, false);
456+
}
457+
458+
/**
459+
* Same as {@link #executeJS(String)}, however
460+
* the code will definitely be run even if the UI currently is loading. <br>
461+
* Note that if you never load the UI this might cause memory leaks or throw a {@link NullPointerException}
462+
* if the UI is null, thus make sure to call this within {@link UI#access(Runnable)} or {@link #now(Consumer)}
463+
* or {@link #later(Consumer)}. <br>
464+
*/
465+
public THIS executeJSForced(String code){
466+
return executeJS(UI.get(), code, true);
445467
}
446468

447469
/**
448470
* @see #executeJS(String)
449471
*/
450472
public THIS executeJS(UI ui, String code){
451-
if(ui == null || ui.isLoading()) return _this;
473+
return executeJS(ui, code, false);
474+
}
475+
476+
/**
477+
* @see #executeJS(String)
478+
*/
479+
public THIS executeJS(UI ui, String code, boolean force){
480+
if(!force && (ui == null || ui.isLoading())) return _this;
452481
if(isAttached){
453482
ui.executeJavaScriptSafely(
454483
"try{"+
@@ -457,7 +486,12 @@ public THIS executeJS(UI ui, String code){
457486
"}catch(e){console.error(e)}",
458487
"internal", 0);
459488
} else{ // Execute code once attached
460-
_onAttached.addOneTimeAction((event, action) -> {
489+
_onAttached.addAction((action, val) -> {
490+
if(!val){
491+
// Detach is not the event we want
492+
return;
493+
}
494+
action.remove();
461495
ui.executeJavaScriptSafely(
462496
"try{"+
463497
ui.jsGetComp("comp", id) +

src/main/java/com/osiris/desku/ui/UI.java

Lines changed: 119 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,13 @@
1212
import org.jsoup.nodes.Document;
1313
import org.jsoup.nodes.Element;
1414

15-
import java.io.File;
16-
import java.io.IOException;
15+
import java.io.*;
1716
import java.lang.reflect.InvocationTargetException;
1817
import java.net.ServerSocket;
1918
import java.nio.charset.StandardCharsets;
2019
import java.nio.file.Files;
2120
import java.util.*;
21+
import java.util.concurrent.CompletableFuture;
2222
import java.util.concurrent.atomic.AtomicBoolean;
2323
import java.util.function.Consumer;
2424

@@ -151,8 +151,8 @@ public static void remove(Thread... threads) {
151151
/**
152152
* @see #executeJavaScriptSafely(String, String, int)
153153
*/
154-
public void executeJavaScriptSafely(String jsCode) {
155-
executeJavaScriptSafely(jsCode, "internal", 0);
154+
public CompletableFuture<Void> executeJavaScriptSafely(String jsCode) {
155+
return executeJavaScriptSafely(jsCode, "internal", 0);
156156
}
157157

158158
/**
@@ -161,8 +161,24 @@ public void executeJavaScriptSafely(String jsCode) {
161161
*
162162
* @see #getSnapshot() internal JS dependencies are added here.
163163
*/
164-
public void executeJavaScriptSafely(String jsCode, String jsCodeSourceName, int jsCodeStartingLineNumber) {
165-
runIfReadyOrLater(() -> executeJavaScript(jsCode, jsCodeSourceName, jsCodeStartingLineNumber));
164+
public CompletableFuture<Void> executeJavaScriptSafely(String jsCode, String jsCodeSourceName, int jsCodeStartingLineNumber) {
165+
var f = new CompletableFuture<Void>();
166+
if(App.isInDepthDebugging) {
167+
Exception e = new Exception();
168+
var sw = new StringWriter();
169+
e.printStackTrace(new PrintWriter(sw));
170+
String finalJsCode = "var javaStackTrace = `"+Value.escapeForJavaScript(sw.toString())+"`;\n\n\n" + jsCode;
171+
runIfReadyOrLater(() -> {
172+
executeJavaScript(finalJsCode, jsCodeSourceName, jsCodeStartingLineNumber);
173+
f.complete(null);
174+
});
175+
} else{
176+
runIfReadyOrLater(() -> {
177+
executeJavaScript(jsCode, jsCodeSourceName, jsCodeStartingLineNumber);
178+
f.complete(null);
179+
});
180+
}
181+
return f;
166182
}
167183

168184
/**
@@ -265,6 +281,9 @@ public UI access(Runnable code) {
265281
for (PendingAppend pendingAppend : pendingAppends) {
266282
try {
267283
attachToParentSafely(pendingAppend);
284+
} catch (InvalidParentException e){
285+
// Do nothing / remove pendingAppend from list
286+
// since its component probably was never added to a parent
268287
} catch (Exception e) {
269288
AL.warn(e);
270289
}
@@ -512,10 +531,10 @@ public void z_internal_load(Class<? extends Route> routeClass) throws IOExceptio
512531
this.route = route;
513532
this.listenersAndComps.clear();
514533
this.content = route.loadContent();
534+
this.content.setAttached(true);
515535
this.content.forEachChildRecursive(child -> {
516536
child.setAttached(true);
517537
});
518-
this.content.setAttached(true);
519538
UI.remove(Thread.currentThread());
520539
}
521540

@@ -680,36 +699,64 @@ public String jsGetComp(String varName, int id) {
680699
}
681700

682701
/**
683-
* Ensures all parents are attached before actually
684-
* performing the pending append operation.
685-
*/
686-
private void attachToParentSafely(PendingAppend pendingAppend) {
687-
if (pendingAppend.child.isAttached()) return;
688-
689-
List<MyElement> parents = new ArrayList<>();
690-
MyElement parent = pendingAppend.parent.element;
691-
while (parent != null && parent instanceof MyElement) {
692-
parents.add(parent); // Make sure that the last attached parent is given too
693-
if (parent.comp.isAttached()) break;
694-
Element p = parent.parent();
695-
if (p instanceof MyElement) parent = (MyElement) p;
696-
else break;
697-
}
698-
if (parents.size() >= 2) {
699-
MyElement rootParentParent = parents.get(parents.size() - 1); // attached
700-
MyElement rootParent = parents.get(parents.size() - 2); // not attached yet
701-
// If this gets appended, there is no need of
702-
// performing the pending append operations of all its children.
703-
rootParent.comp.updateAll();
704-
attachToParent(rootParentParent.comp, rootParent.comp,
705-
new Component.AddedChildEvent(rootParent.comp, null, false, false));
706-
// Above also sets isAttached = true of all child components recursively,
707-
// thus next attachToParentSafely() will return directly without doing nothing,
708-
// and thus all pending appends for those children will not be executed,
709-
// since otherwise that would cause duplicate components
710-
} else {
702+
* Ensures all parents are attached before performing the pending append operation.
703+
*/
704+
private CompletableFuture<Void> attachToParentSafely(PendingAppend pendingAppend) throws InvalidParentException {
705+
if (pendingAppend.child.isAttached()) {
706+
return CompletableFuture.completedFuture(null);
707+
}
708+
709+
List<Component> parents = getParentChain(pendingAppend.parent);
710+
if(App.isInDepthDebugging){
711+
AL.info("Unattached parent chain ("+parents.size()+") child -> parent: ");
712+
String s = "";
713+
for (Component comp : parents) {
714+
s += comp.toPrintString()+" isAttached="+comp.isAttached()+" ||| ";
715+
}
716+
AL.debug(this.getClass(), s);
717+
}
718+
719+
if (parents.size() < 2) {
711720
pendingAppend.child.updateAll();
712-
attachToParent(pendingAppend.parent, pendingAppend.child, pendingAppend.e);
721+
return attachToParent(pendingAppend.parent, pendingAppend.child, pendingAppend.e);
722+
}
723+
724+
Component rootParentParent = parents.get(parents.size() - 1); // attached
725+
Component rootParent = parents.get(parents.size() - 2); // not attached yet
726+
727+
rootParent.updateAll();
728+
return attachToParent(rootParentParent, rootParent,
729+
new Component.AddedChildEvent(rootParent, null, false, false));
730+
}
731+
732+
/**
733+
* Returns a list of mainly unattached parents (the last element is the first attached parent that is found), starting from the given comp and moving up the hierarchy.
734+
* The list is in order from child to parent. <br>
735+
* <br>
736+
* @throws InvalidParentException if the last comp is not attached. Meaning the component probably was never added to a parent on the Java side.
737+
*/
738+
private List<Component> getParentChain(Component startElement) throws InvalidParentException {
739+
List<Component> parents = new ArrayList<>();
740+
MyElement current = startElement.element;
741+
742+
while (current != null && current instanceof MyElement) {
743+
parents.add(current.comp);
744+
if (current.comp.isAttached()) break;
745+
746+
Element parent = current.parent();
747+
if (parent == null || !(parent instanceof MyElement)){
748+
throw new InvalidParentException("Parent is invalid/null, possibly meaning that the component was never added to a parent on the Java side." +
749+
"However it can also mean that update() was never called before. comp="+current.comp.toPrintString()+" parent="+parent+" ");
750+
}
751+
current = (MyElement) parent;
752+
}
753+
754+
return parents;
755+
}
756+
757+
public class InvalidParentException extends Exception{
758+
public InvalidParentException(String message) {
759+
super(message);
713760
}
714761
}
715762

@@ -725,30 +772,35 @@ public void attachWhenAccessEnds(Component<?, ?> parent, Component<?, ?> child,
725772
}
726773
}
727774

728-
public <T extends Component<?, ?>> void attachToParent(Component<?, ?> parent, Component<?, ?> child, Component.AddedChildEvent e) {
775+
public <T extends Component<?, ?>> CompletableFuture<Void> attachToParent(Component<?, ?> parent, Component<?, ?> child, Component.AddedChildEvent e) {
729776
if(App.isInDepthDebugging) AL.debug(this.getClass(), "attachToParent() parent = "+parent.toPrintString()+" attached="+parent.isAttached()+" added child = "+
730777
child.toPrintString()+" child html = \n"+ child.element.outerHtml());
778+
if(!parent.isAttached())
779+
throw new RuntimeException("Attempting attach to parent even though parent '"+parent.toPrintString()+"' is not attached yet!");
731780

732781
if (e.otherChildComp == null) { // add
733-
executeJavaScript(jsAttachToParent(parent, child),
734-
"internal", 0);
735-
child.setAttached(true);
736-
child.forEachChildRecursive(child2 -> {
737-
child2.setAttached(true);
782+
return executeJavaScriptSafely(jsAttachToParent(parent, child),
783+
"internal", 0).thenAccept(__ -> {
784+
child.setAttached(true);
785+
child.forEachChildRecursive(child2 -> {
786+
child2.setAttached(true);
787+
});
738788
});
739-
} else if (e.isInsert || e.isReplace) { // for replace, remove() must be executed after this function returns
789+
} else { //if (e.isInsert || e.isReplace) { // for replace, remove() must be executed after this function returns
740790
// if replace: childComp is the new component to be added and otherChildComp is the one that gets removed/replaced
741791
// "beforebegin" = Before the element. Only valid if the element is in the DOM tree and has a parent element.
742-
executeJavaScript(
792+
return executeJavaScriptSafely(
743793
jsGetComp("otherChildComp", e.otherChildComp.id) +
744794
"var child = `" + Value.escapeForJavaScript(Value.escapeForJSON(e.childComp.element.outerHtml())) + "`;\n" +
745795
"otherChildComp.insertAdjacentHTML(\"beforebegin\", child);\n" +
746796
(App.isInDepthDebugging ? "console.log('otherChildComp:', otherChildComp); console.log('➡️✅ inserted child:', child); \n" : ""),
747-
"internal", 0);
748-
e.childComp.setAttached(true);
749-
e.childComp.forEachChildRecursive(child2 -> {
750-
child2.setAttached(true);
751-
});
797+
"internal", 0)
798+
.thenAccept(__ -> {
799+
e.childComp.setAttached(true);
800+
e.childComp.forEachChildRecursive(child2 -> {
801+
child2.setAttached(true);
802+
});
803+
});
752804
}
753805
}
754806

@@ -789,13 +841,29 @@ public boolean isLoading(){
789841
}
790842

791843
public static class PendingAppend {
844+
/**
845+
* Also removes all children recursively.
846+
*/
792847
public static boolean removeFromPendingAppends(UI ui, Component<?, ?> comp){
793848
if(ui != null){ // && ui.isLoading()){ // Also remove from pending appends, these can also happen after loading
794849
synchronized (ui.pendingAppends){
795850
List<UI.PendingAppend> toRemove = new ArrayList<>(0);
796-
for (UI.PendingAppend pendingAppend : ui.pendingAppends) {
797-
if(comp.equals(pendingAppend.child))
851+
for (UI.PendingAppend pendingAppend : ui.getPendingAppendsCopy()) {
852+
if(comp.equals(pendingAppend.child)){
798853
toRemove.add(pendingAppend);
854+
// Also remove any children
855+
for (Component child : comp.children) {
856+
removeFromPendingAppends(ui, child);
857+
}
858+
}
859+
}
860+
if(App.isInDepthDebugging){
861+
if(!toRemove.isEmpty()){
862+
AL.info("Removing "+toRemove.size()+" components from pending append:");
863+
for (PendingAppend pa : toRemove) {
864+
AL.info("toRemove = " + pa.child.toPrintString());
865+
}
866+
}
799867
}
800868
return ui.pendingAppends.removeAll(toRemove);
801869
}

src/main/java/com/osiris/desku/ui/input/filechooser/FileChooser.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ private void setButtons(List<File> files){
111111
private Button getButton(FileAsRow e) {
112112
return new Button(e.cleanFilePath).onClick(e2 -> {
113113
setDir(e.file.isDirectory() ? e.file : e.file.getParentFile());
114+
directoryView.visible(!directoryView.isVisible());
114115
});
115116
}
116117

src/main/java/com/osiris/desku/ui/layout/Tooltip.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package com.osiris.desku.ui.layout;
22

33
import com.osiris.desku.ui.Component;
4-
import com.osiris.desku.ui.UI;
54

65
public class Tooltip{
76
public Component<?, ?> parent;
@@ -16,8 +15,7 @@ public Tooltip attachToParent(){
1615
parent.atr("data-bs-toggle", "tooltip");
1716
parent.atr("data-bs-title", content);
1817

19-
UI ui = UI.get();
20-
ui.executeJavaScriptSafely(ui.jsGetComp("comp", parent.id) + "new bootstrap.Tooltip(comp)");
18+
parent.executeJSForced("new bootstrap.Tooltip(comp)");
2119
return this;
2220
}
2321
}

src/test/java/com/osiris/desku/bugs/FileChooserTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ public class FileChooserTest {
2020

2121
@Test
2222
void test() throws Throwable {
23-
TApp.testAndAwaitResult((asyncResult) -> {
23+
TApp.testIndefinetely((asyncResult) -> {
2424
FileChooser c = new FileChooser("FC1");
2525
assertEquals("", c.getValue());
2626

0 commit comments

Comments
 (0)