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

New UI main page #98

Merged
merged 24 commits into from
Aug 6, 2020
Merged

New UI main page #98

merged 24 commits into from
Aug 6, 2020

Conversation

amclain
Copy link
Member

@amclain amclain commented Aug 3, 2020

Resolves #78

Adjustments & Notes

  • The design for the list of animation types is currently in flux, so exposing a mechanism to retrieve their names in the UI would add code that we would need to refactor. To avoid this, I have kept the buttons to cycle to the previous/next animation in this iteration.
  • I have avoided creating a custom options list for the select box because it was not possible with pure CSS and according to tailwind, "Custom select controls like this require a considerable amount of JS to implement from scratch. We're planning to build some low-level libraries to make this easier with popular frameworks [...]".link It may be simpler to wait for the tailwind helper library to be released before attempting this.
  • I have omitted the menu icon in the nav bar because there are no other pages to navigate to yet.
  • I have omitted the IP address of the hardware in the UI because it may be unnecessary information. The browser connected via xebow.local, and if the connection were down there would be no way to get to the UI to see this information.

Screenshot

image

@amclain amclain added the feature A new feature or enhancement to an existing feature label Aug 3, 2020
@amclain amclain requested review from doughsay and vanvoljg August 3, 2020 00:26
@amclain amclain self-assigned this Aug 3, 2020
@@ -0,0 +1,179 @@
/*!
Lato font.
Copy link
Member Author

Choose a reason for hiding this comment

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

I copied in the entire lato.scss file, made the necessary changes for this project, and commented out the fonts we aren't using. As much as I try to avoid committing commented code, this should be at low-risk of changing and sets the next developer up to only have to uncomment the code if we add another lato font to the project (like italics).

Copy link
Member

Choose a reason for hiding this comment

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

I'm pretty okay with it. There's not a lack of space in these devices, so this is fine.

@@ -0,0 +1,87 @@
/* LiveView specific classes for your customizations */
Copy link
Member Author

Choose a reason for hiding this comment

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

Do we need to keep the boilerplate live view styling around, or should I delete it?

Choose a reason for hiding this comment

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

You probably want to keep or modify the styling for the feedback, phx-click-loading, phx-disconnected, and alert classes.

Copy link
Member Author

Choose a reason for hiding this comment

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

👍 Thanks!

@amclain amclain force-pushed the feature/ui-main-page branch from 7a5726f to 878fc5e Compare August 3, 2020 01:18
@amclain
Copy link
Member Author

amclain commented Aug 3, 2020

I'm going to hold off on updating this branch to master until #97 is in to avoid merge commit spam.

Copy link
Member

@vanvoljg vanvoljg left a comment

Choose a reason for hiding this comment

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

Thank you, this looks great!

I left a couple tiny comments, nothing blocking.

There's something in the styling that shifts the keypad to the right when it renders the configuration section. I don't know if that's intentional, but it's worth noting to fix for later.

@@ -0,0 +1,179 @@
/*!
Lato font.
Copy link
Member

Choose a reason for hiding this comment

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

I'm pretty okay with it. There's not a lack of space in these devices, so this is fine.

Co-authored-by: Jesse Van Volkinburg <42327429+vanvoljg@users.noreply.github.com>
Copy link
Member

@doughsay doughsay left a comment

Choose a reason for hiding this comment

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

💯

@vanvoljg vanvoljg merged commit 1c295ce into master Aug 6, 2020
@vanvoljg vanvoljg deleted the feature/ui-main-page branch August 6, 2020 02:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature A new feature or enhancement to an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UI: Main page
4 participants