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.

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

Frontend errors #94

Open
wants to merge 30 commits into
base: dev
Choose a base branch
from
Open

Frontend errors #94

wants to merge 30 commits into from

Conversation

Aito0
Copy link
Collaborator

@Aito0 Aito0 commented Feb 20, 2025

Implemented dill exception structure and popup displaying

josh-ja-walker and others added 27 commits February 12, 2025 14:27
@Aito0 Aito0 requested review from aniket1101 and josh-ja-walker and removed request for aniket1101 February 20, 2025 14:00
@Aito0 Aito0 requested a review from AdamW1087 February 20, 2025 14:00
Copy link
Collaborator

@aniket1101 aniket1101 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i dont like this

@aniket1101 aniket1101 self-requested a review February 20, 2025 20:30
@Aito0 Aito0 removed the request for review from aniket1101 February 21, 2025 14:17

val root: HtmlElement = div(
MainView(),
ErrorController.displayError
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a "," on the end, we do it for other places

Comment on lines 23 to 26
val root: HtmlElement = div(
MainView(),
ErrorController.displayError
)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also maybe put root above main? It looks off calling something that is defined below

}
}

/**
* Decode a JSON string into a list of DebugNodes.
*
* @param jsonString The JSON string to convert.
*
*
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whitespace addition :(

}

object MalformedJSONException extends Exception
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be at the bottom, would it be more suitable near the top?

Comment on lines 12 to 13
* ErrorController keeps track of the error state of the app and the code to display error
* popups in the app
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

balance these lines a bit more its really top heavy

setInput (
h1(
className := "debug-tree-title",
p("Parser Input : ", margin.px := 0, fontSize.px := 15,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make css obj for the text?

@@ -24,6 +30,8 @@ object MainViewController {
def getMainView: Signal[HtmlElement] =
isTreeView.signal.map(if (_) then TreeView() else InputView())

def getNoTreeFound: HtmlElement = noTreeFound
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment?

Comment on lines +22 to +38
def displayElement: HtmlElement = {
div(
className := "popup",
cls := style,
h2(
this.name,
color := "#ffffff",
),
br(),
div(
className := "popup-text",
this.message,
),
if closable then onClick --> {_ => ErrorController.clearError()} else onClick --> {_ => ()}
)
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

brace not needed

Comment on lines 24 to 27
def apply(): HtmlElement = div(
TreeViewController.getDisplayTree, /* Renders the tree */
)
def apply(): HtmlElement =
div(
TreeViewController.getDisplayTree /* Renders the tree */
)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-inline?

styles.css Outdated
@@ -16,6 +17,7 @@
--font-color-primary: #96DEC4; /* Parsley green. */
--font-color-secondary: #242526; /* Dark grey. */
--font-color-error: #d2042d; /* Red error. */
--font-color-warning: #ffcc00; /* Yelow warning colour. */
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yellow typo

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

riley ahh yelow

Comment on lines 23 to 25
val root: HtmlElement = div(
MainView(),
ErrorController.displayError
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move to MainView

* @param input The input string
*/
def toInputElement(input: String): Unit =
setInput (
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bracez

Comment on lines 44 to 56
override def colour: String = "ffff00"
override def closable: Boolean = true
override def style: String = "warning"
}

/**
* Represents a breaking Exception in Dill. Overrides colour to red and canDelete to false
*/
sealed trait Error extends DillException {
override def colour: String = "ff0000"
override def closable: Boolean = false
override def style: String = "error"
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

colour is set in css

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so remove this

}

/* Used when an unexpected error occurs */
case class UnknownError(mes: String) extends Error {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

msg instead of mes

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please dont ignore this

styles.css Outdated
@@ -16,6 +17,7 @@
--font-color-primary: #96DEC4; /* Parsley green. */
--font-color-secondary: #242526; /* Dark grey. */
--font-color-error: #d2042d; /* Red error. */
--font-color-warning: #ffcc00; /* Yelow warning colour. */
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

riley ahh yelow

@@ -16,6 +17,7 @@
--font-color-primary: #96DEC4; /* Parsley green. */
--font-color-secondary: #242526; /* Dark grey. */
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dark grey comment not lined up with the others

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still not lined up

@@ -14,27 +19,26 @@ object DebugTreeController {
* Decode a JSON string into a DebugTree class.
*
* @param jsonString The JSON string to convert.
*
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please bring back :(

Comment on lines +112 to +115
"Failed to serialize the display tree. " +
"This may indicate an issue with the serialization process. " +
"Please report this issue to Jamie Willis if the problem persists"
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NOT TO JAMIE

}

/* Used when an unexpected error occurs */
case class UnknownError(mes: String) extends Error {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please dont ignore this

@@ -16,6 +17,7 @@
--font-color-primary: #96DEC4; /* Parsley green. */
--font-color-secondary: #242526; /* Dark grey. */
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still not lined up

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants