Skip to content

Commit

Permalink
This patch includes updates to multiple Java classes within the `com.…
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
Osiris-Team committed Aug 13, 2024
1 parent 1e1c67e commit 921a43d
Show file tree
Hide file tree
Showing 5 changed files with 162 additions and 61 deletions.
46 changes: 40 additions & 6 deletions src/main/java/com/osiris/desku/ui/Component.java
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,15 @@ public class Component<THIS extends Component<THIS, VALUE>, VALUE> {
* </pre>
*/
public final int id = idCounter.getAndIncrement();
/**
* Always null, unless {@link App#isInDepthDebugging} is enabled. <br>
* Stacktrace to find where in your code this component was created. <br>
*/
public StackTraceElement[] createdAtStracktrace = null;
{
if(App.isInDepthDebugging)
createdAtStracktrace = new Exception().getStackTrace();
}
/**
* List of children. Normally it's read-only. <br>
* Thus do not modfify directly and use methods like {@link #add(Component[])} or {@link #remove(Component[])}
Expand Down Expand Up @@ -105,9 +114,9 @@ public class Component<THIS extends Component<THIS, VALUE>, VALUE> {
public final Event<ValueChangeEvent<THIS, VALUE>> readOnlyOnValueChange = new Event<>();
/**
* Gets executed when this component <br>
* was attached to the UI.
* was attached or detached from the UI.
*/
public final Event<Void> _onAttached = new Event<>();
public final Event<Boolean> _onAttached = new Event<>();
public final ConcurrentHashMap<String, String> style = new ConcurrentHashMap<>();
/**
* Gets set to false in {@link AddedChildEvent}. <br>
Expand All @@ -121,7 +130,7 @@ public class Component<THIS extends Component<THIS, VALUE>, VALUE> {

public THIS setAttached(boolean b){
isAttached = b;
_onAttached.execute(null);
_onAttached.execute(b);
return _this;
}
public boolean isAttached(){
Expand Down Expand Up @@ -149,6 +158,8 @@ public boolean isAttached(){
*/
public MyElement element;
public Consumer<Component> _remove = child -> {
if(App.isInDepthDebugging)
AL.debug(this.getClass(), "Attempting to remove child '"+child.toPrintString()+"' from parent '"+this.toPrintString()+"'.");
UI ui = UI.get(); // Necessary for updating the actual UI via JavaScript
if (children.contains(child)) {
children.remove(child);
Expand Down Expand Up @@ -441,14 +452,32 @@ public boolean isValuesEqual(VALUE val1, VALUE val2){
* this component via the "comp" variable in your provided JavaScript code.
*/
public THIS executeJS(String code){
return executeJS(UI.get(), code);
return executeJS(UI.get(), code, false);
}

/**
* Same as {@link #executeJS(String)}, however
* the code will definitely be run even if the UI currently is loading. <br>
* Note that if you never load the UI this might cause memory leaks or throw a {@link NullPointerException}
* if the UI is null, thus make sure to call this within {@link UI#access(Runnable)} or {@link #now(Consumer)}
* or {@link #later(Consumer)}. <br>
*/
public THIS executeJSForced(String code){
return executeJS(UI.get(), code, true);
}

/**
* @see #executeJS(String)
*/
public THIS executeJS(UI ui, String code){
if(ui == null || ui.isLoading()) return _this;
return executeJS(ui, code, false);
}

/**
* @see #executeJS(String)
*/
public THIS executeJS(UI ui, String code, boolean force){
if(!force && (ui == null || ui.isLoading())) return _this;
if(isAttached){
ui.executeJavaScriptSafely(
"try{"+
Expand All @@ -457,7 +486,12 @@ public THIS executeJS(UI ui, String code){
"}catch(e){console.error(e)}",
"internal", 0);
} else{ // Execute code once attached
_onAttached.addOneTimeAction((event, action) -> {
_onAttached.addAction((action, val) -> {
if(!val){
// Detach is not the event we want
return;
}
action.remove();
ui.executeJavaScriptSafely(
"try{"+
ui.jsGetComp("comp", id) +
Expand Down
170 changes: 119 additions & 51 deletions src/main/java/com/osiris/desku/ui/UI.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@
import org.jsoup.nodes.Document;
import org.jsoup.nodes.Element;

import java.io.File;
import java.io.IOException;
import java.io.*;
import java.lang.reflect.InvocationTargetException;
import java.net.ServerSocket;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.util.*;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Consumer;

Expand Down Expand Up @@ -151,8 +151,8 @@ public static void remove(Thread... threads) {
/**
* @see #executeJavaScriptSafely(String, String, int)
*/
public void executeJavaScriptSafely(String jsCode) {
executeJavaScriptSafely(jsCode, "internal", 0);
public CompletableFuture<Void> executeJavaScriptSafely(String jsCode) {
return executeJavaScriptSafely(jsCode, "internal", 0);
}

/**
Expand All @@ -161,8 +161,24 @@ public void executeJavaScriptSafely(String jsCode) {
*
* @see #getSnapshot() internal JS dependencies are added here.
*/
public void executeJavaScriptSafely(String jsCode, String jsCodeSourceName, int jsCodeStartingLineNumber) {
runIfReadyOrLater(() -> executeJavaScript(jsCode, jsCodeSourceName, jsCodeStartingLineNumber));
public CompletableFuture<Void> executeJavaScriptSafely(String jsCode, String jsCodeSourceName, int jsCodeStartingLineNumber) {
var f = new CompletableFuture<Void>();
if(App.isInDepthDebugging) {
Exception e = new Exception();
var sw = new StringWriter();
e.printStackTrace(new PrintWriter(sw));
String finalJsCode = "var javaStackTrace = `"+Value.escapeForJavaScript(sw.toString())+"`;\n\n\n" + jsCode;
runIfReadyOrLater(() -> {
executeJavaScript(finalJsCode, jsCodeSourceName, jsCodeStartingLineNumber);
f.complete(null);
});
} else{
runIfReadyOrLater(() -> {
executeJavaScript(jsCode, jsCodeSourceName, jsCodeStartingLineNumber);
f.complete(null);
});
}
return f;
}

/**
Expand Down Expand Up @@ -265,6 +281,9 @@ public UI access(Runnable code) {
for (PendingAppend pendingAppend : pendingAppends) {
try {
attachToParentSafely(pendingAppend);
} catch (InvalidParentException e){
// Do nothing / remove pendingAppend from list
// since its component probably was never added to a parent
} catch (Exception e) {
AL.warn(e);
}
Expand Down Expand Up @@ -512,10 +531,10 @@ public void z_internal_load(Class<? extends Route> routeClass) throws IOExceptio
this.route = route;
this.listenersAndComps.clear();
this.content = route.loadContent();
this.content.setAttached(true);
this.content.forEachChildRecursive(child -> {
child.setAttached(true);
});
this.content.setAttached(true);
UI.remove(Thread.currentThread());
}

Expand Down Expand Up @@ -680,36 +699,64 @@ public String jsGetComp(String varName, int id) {
}

/**
* Ensures all parents are attached before actually
* performing the pending append operation.
*/
private void attachToParentSafely(PendingAppend pendingAppend) {
if (pendingAppend.child.isAttached()) return;

List<MyElement> parents = new ArrayList<>();
MyElement parent = pendingAppend.parent.element;
while (parent != null && parent instanceof MyElement) {
parents.add(parent); // Make sure that the last attached parent is given too
if (parent.comp.isAttached()) break;
Element p = parent.parent();
if (p instanceof MyElement) parent = (MyElement) p;
else break;
}
if (parents.size() >= 2) {
MyElement rootParentParent = parents.get(parents.size() - 1); // attached
MyElement rootParent = parents.get(parents.size() - 2); // not attached yet
// If this gets appended, there is no need of
// performing the pending append operations of all its children.
rootParent.comp.updateAll();
attachToParent(rootParentParent.comp, rootParent.comp,
new Component.AddedChildEvent(rootParent.comp, null, false, false));
// Above also sets isAttached = true of all child components recursively,
// thus next attachToParentSafely() will return directly without doing nothing,
// and thus all pending appends for those children will not be executed,
// since otherwise that would cause duplicate components
} else {
* Ensures all parents are attached before performing the pending append operation.
*/
private CompletableFuture<Void> attachToParentSafely(PendingAppend pendingAppend) throws InvalidParentException {
if (pendingAppend.child.isAttached()) {
return CompletableFuture.completedFuture(null);
}

List<Component> parents = getParentChain(pendingAppend.parent);
if(App.isInDepthDebugging){
AL.info("Unattached parent chain ("+parents.size()+") child -> parent: ");
String s = "";
for (Component comp : parents) {
s += comp.toPrintString()+" isAttached="+comp.isAttached()+" ||| ";
}
AL.debug(this.getClass(), s);
}

if (parents.size() < 2) {
pendingAppend.child.updateAll();
attachToParent(pendingAppend.parent, pendingAppend.child, pendingAppend.e);
return attachToParent(pendingAppend.parent, pendingAppend.child, pendingAppend.e);
}

Component rootParentParent = parents.get(parents.size() - 1); // attached
Component rootParent = parents.get(parents.size() - 2); // not attached yet

rootParent.updateAll();
return attachToParent(rootParentParent, rootParent,
new Component.AddedChildEvent(rootParent, null, false, false));
}

/**
* 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.
* The list is in order from child to parent. <br>
* <br>
* @throws InvalidParentException if the last comp is not attached. Meaning the component probably was never added to a parent on the Java side.
*/
private List<Component> getParentChain(Component startElement) throws InvalidParentException {
List<Component> parents = new ArrayList<>();
MyElement current = startElement.element;

while (current != null && current instanceof MyElement) {
parents.add(current.comp);
if (current.comp.isAttached()) break;

Element parent = current.parent();
if (parent == null || !(parent instanceof MyElement)){
throw new InvalidParentException("Parent is invalid/null, possibly meaning that the component was never added to a parent on the Java side." +
"However it can also mean that update() was never called before. comp="+current.comp.toPrintString()+" parent="+parent+" ");
}
current = (MyElement) parent;
}

return parents;
}

public class InvalidParentException extends Exception{
public InvalidParentException(String message) {
super(message);
}
}

Expand All @@ -725,30 +772,35 @@ public void attachWhenAccessEnds(Component<?, ?> parent, Component<?, ?> child,
}
}

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

if (e.otherChildComp == null) { // add
executeJavaScript(jsAttachToParent(parent, child),
"internal", 0);
child.setAttached(true);
child.forEachChildRecursive(child2 -> {
child2.setAttached(true);
return executeJavaScriptSafely(jsAttachToParent(parent, child),
"internal", 0).thenAccept(__ -> {
child.setAttached(true);
child.forEachChildRecursive(child2 -> {
child2.setAttached(true);
});
});
} else if (e.isInsert || e.isReplace) { // for replace, remove() must be executed after this function returns
} else { //if (e.isInsert || e.isReplace) { // for replace, remove() must be executed after this function returns
// if replace: childComp is the new component to be added and otherChildComp is the one that gets removed/replaced
// "beforebegin" = Before the element. Only valid if the element is in the DOM tree and has a parent element.
executeJavaScript(
return executeJavaScriptSafely(
jsGetComp("otherChildComp", e.otherChildComp.id) +
"var child = `" + Value.escapeForJavaScript(Value.escapeForJSON(e.childComp.element.outerHtml())) + "`;\n" +
"otherChildComp.insertAdjacentHTML(\"beforebegin\", child);\n" +
(App.isInDepthDebugging ? "console.log('otherChildComp:', otherChildComp); console.log('➡️✅ inserted child:', child); \n" : ""),
"internal", 0);
e.childComp.setAttached(true);
e.childComp.forEachChildRecursive(child2 -> {
child2.setAttached(true);
});
"internal", 0)
.thenAccept(__ -> {
e.childComp.setAttached(true);
e.childComp.forEachChildRecursive(child2 -> {
child2.setAttached(true);
});
});
}
}

Expand Down Expand Up @@ -789,13 +841,29 @@ public boolean isLoading(){
}

public static class PendingAppend {
/**
* Also removes all children recursively.
*/
public static boolean removeFromPendingAppends(UI ui, Component<?, ?> comp){
if(ui != null){ // && ui.isLoading()){ // Also remove from pending appends, these can also happen after loading
synchronized (ui.pendingAppends){
List<UI.PendingAppend> toRemove = new ArrayList<>(0);
for (UI.PendingAppend pendingAppend : ui.pendingAppends) {
if(comp.equals(pendingAppend.child))
for (UI.PendingAppend pendingAppend : ui.getPendingAppendsCopy()) {
if(comp.equals(pendingAppend.child)){
toRemove.add(pendingAppend);
// Also remove any children
for (Component child : comp.children) {
removeFromPendingAppends(ui, child);
}
}
}
if(App.isInDepthDebugging){
if(!toRemove.isEmpty()){
AL.info("Removing "+toRemove.size()+" components from pending append:");
for (PendingAppend pa : toRemove) {
AL.info("toRemove = " + pa.child.toPrintString());
}
}
}
return ui.pendingAppends.removeAll(toRemove);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ private void setButtons(List<File> files){
private Button getButton(FileAsRow e) {
return new Button(e.cleanFilePath).onClick(e2 -> {
setDir(e.file.isDirectory() ? e.file : e.file.getParentFile());
directoryView.visible(!directoryView.isVisible());
});
}

Expand Down
4 changes: 1 addition & 3 deletions src/main/java/com/osiris/desku/ui/layout/Tooltip.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package com.osiris.desku.ui.layout;

import com.osiris.desku.ui.Component;
import com.osiris.desku.ui.UI;

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

UI ui = UI.get();
ui.executeJavaScriptSafely(ui.jsGetComp("comp", parent.id) + "new bootstrap.Tooltip(comp)");
parent.executeJSForced("new bootstrap.Tooltip(comp)");
return this;
}
}
2 changes: 1 addition & 1 deletion src/test/java/com/osiris/desku/bugs/FileChooserTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public class FileChooserTest {

@Test
void test() throws Throwable {
TApp.testAndAwaitResult((asyncResult) -> {
TApp.testIndefinetely((asyncResult) -> {
FileChooser c = new FileChooser("FC1");
assertEquals("", c.getValue());

Expand Down

0 comments on commit 921a43d

Please sign in to comment.