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

Flatten RedwoodView and RedwoodView.Root #2391

Merged
merged 5 commits into from
Oct 21, 2024
Merged

Conversation

squarejesse
Copy link
Contributor

This is a step in simplifying the complex interaction between these two classes. It also removes an unnecessary layer from the view tree.

We also had a potential bug on hot reloads, where the previous content was detached but not necessarily removed. With this change we clear the child views by default on a hot reload.


  • CHANGELOG.md's "Unreleased" section has been updated, if applicable.

This is a step in simplifying the complex interaction between these
two classes. It also removes an unnecessary layer from the view
tree.

We also had a potential bug on hot reloads, where the previous content
was detached but not necessarily removed. With this change we clear
the child views by default on a hot reload.
if (child is ExceptionView || child is LoadingView) {
i.remove()
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let the base class just clear everything

loadCount: Int,
attached: Boolean,
uncaughtException: Throwable? = null,
)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For a follow-up, I’ll refactor these arguments to match what the content can actually be:

sealed class ContentState {
  /** 
   * This view has no content. This is the initial state of the view.
   * 
   * This may occur if the content is gracefully stopped, in which case
   * the view may continue to display the previous content, though it
   * will not be interactive.
   */
  object Detached : ContentState

  /** 
   * Content isn't ready yet. It’s busy downloading, launching, or computing 
   * the initial view tree.
   */
  object Loading : ContentState()

  /** 
   * Content is ready and responsive to interactions. 
   */
  data class Attached(val loadCount: Int): ContentState()

  /** 
   * This view has crashed due to a bug.
   */
  data class Crashed(uncaughtException: Throwable?): ContentState()
}

@squarejesse squarejesse enabled auto-merge (squash) October 21, 2024 17:19
@squarejesse squarejesse merged commit 12420a5 into trunk Oct 21, 2024
11 checks passed
@squarejesse squarejesse deleted the jwilson.1021.flatten branch October 21, 2024 20:26
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.

2 participants