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

Allow port configuration, refactor start actor modal #73

Merged
merged 6 commits into from Jun 30, 2021

Conversation

brooksmtownsend
Copy link
Member

This started as just a port configuration PR, but with some more UI changes with actors I figured I'd give refactoring the modal a try.

Along with port configuration, this PR also shows the appropriate host ID in the actor table, and the new "homepage" supposed to be the mockup for the full lattice view. In subsequent PRs, the lattice view will show a list of hosts that can be further inspected for only the resources (actors, providers) running on that specific host.

What I'd love feedback on is the code for the start actor component, how it's laid out in the page_live template, and how it looks when being used. Refactoring to live components makes it so that the modal disappears and only the table re-renders on the page, instead of a full POST request + page reload. I'd like to do the same thing to the define-link modal and the start providers modals, but I'd like some feedback before putting in the effort (and potentially mistakes) on those.

some actor modal cleanup

format

remove unused template

quick refactor of start_actor_file signature
Comment on lines 167 to 173
/**
* For some reason, stopping the execution of this file here makes the charts load correctly.
* Until we figure that out, leave this in.
* Thanks, JavaScript.
*/
null.ignoreThisError No newline at end of file
// /**
// * For some reason, stopping the execution of this file here makes the charts load correctly.
// * Until we figure that out, leave this in.
// * Thanks, JavaScript.
// */
// null.ignoreThisError
Copy link
Member Author

Choose a reason for hiding this comment

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

As fun as this hack was, it does break the live view reloading, so the charts are going to have to be less impressive for a short time. They still render, just in a way that seems a little incorrect, so we can open up an issue to address that once we tackle #45

Copy link
Member

@autodidaddict autodidaddict left a comment

Choose a reason for hiding this comment

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

Only changes I suggest is removing the commented out JavaScript.

Copy link
Member

@autodidaddict autodidaddict left a comment

Choose a reason for hiding this comment

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

I know they're placeholders, but just change the info logger for handling the health check pass/fail.

@brooksmtownsend brooksmtownsend merged commit 11bce2c into main Jun 30, 2021
@brooksmtownsend brooksmtownsend deleted the ahoy-matey-open-up-the-ports branch June 30, 2021 13:25
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.

None yet

2 participants