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

Review + Expand Instructions #42

Closed
5 of 7 tasks
nelsonic opened this issue Jun 1, 2022 · 19 comments
Closed
5 of 7 tasks

Review + Expand Instructions #42

nelsonic opened this issue Jun 1, 2022 · 19 comments
Assignees
Labels
awaiting-review An issue or pull request that needs to be reviewed chore a tedious but necessary task often paying technical debt enhancement New feature or enhancement of existing functionality good first issue Good for newcomers help wanted If you can help make progress with this issue, please comment! priority-2 Second highest priority, should be worked on as soon as the Priority-1 issues are finished T4h Time Estimate 4 Hours

Comments

@nelsonic
Copy link
Member

nelsonic commented Jun 1, 2022

Context

We are using LiveView for a Client project.
The project already has some basic LiveView UI but has barely scratched the surface of what's possible.
Specifically I want to add "who is logged in" (presence) to our gitea-demo App's "advanced" example: dwyl/gitea-demo#12

This LiveView Chat Example App is already an excellent starting point for understanding how to do this.
But I feel that as a complete beginner a couple of steps can be expanded. 💭

Todo

Going to spend T4h on this tomorrow morning as I want to reference it in the work I'm doing in dwyl/gitea-demo#12

Will assign PR once ready. 👍

@nelsonic nelsonic added enhancement New feature or enhancement of existing functionality good first issue Good for newcomers help wanted If you can help make progress with this issue, please comment! T4h Time Estimate 4 Hours priority-2 Second highest priority, should be worked on as soon as the Priority-1 issues are finished chore a tedious but necessary task often paying technical debt labels Jun 1, 2022
@nelsonic nelsonic self-assigned this Jun 1, 2022
@nelsonic
Copy link
Member Author

nelsonic commented Jun 2, 2022

Took a minor detour to investigate using Tailwind ... https://fly.io/phoenix-files/tailwind-standalone/ 💭

@nelsonic nelsonic added the in-progress An issue or pull request that is being worked on by the assigned person label Jun 4, 2022
@nelsonic
Copy link
Member Author

nelsonic commented Jun 4, 2022

Technology stacks are like Automobile Brands. There are dozens of them.
None are "right" or "wrong". They all have pros and cons. Costs and benefits.

image

Note: This graphic illustrates the "big" manufacturers/brands
and doesn't include brands you have definitely heard of such as Mazda, Subaru or Susuki
or smaller/niche brands like Ferrari, McLaren, Morgan and Koenigsegg
or newer brands like BYD, Fisker, Lucid, NIO, XPeng, Rivian & Tesla ...
Bottom line: there are many ways of building vehicles.
Equally there are many ways to build software and especially web-based software.

@SimonLab
Copy link
Member

SimonLab commented Jun 6, 2022

Looking at the failing tests, the error is linked to auth_plug where the api_key environment variable is not defined
Needed to make sure the AUTH_API_KEY is sourced before running tests.
PR ready: #45

@nelsonic
Copy link
Member Author

nelsonic commented Jun 7, 2022

The build passed on Simon's PR #45
But fails on main:
https://github.com/dwyl/phoenix-liveview-chat-example/runs/6757621874?check_suite_focus=true#step:7:140
image

going to try and re-run it just to see if it's an intermittent failure or perm one. ♻️

@nelsonic
Copy link
Member Author

nelsonic commented Jun 7, 2022

Still fails:
image

https://github.com/dwyl/phoenix-liveview-chat-example/runs/6772512125?check_suite_focus=true#step:7:137

image

Lame. Looks like an issue in AuthPlug we can investigate this later. 🤞

@SimonLab
Copy link
Member

SimonLab commented Jun 7, 2022

The timeout issue is due to the auth Heroku app not being awake for the login test I think. We could mock this call to Auth maybe

@nelsonic
Copy link
Member Author

nelsonic commented Jun 7, 2022

@SimonLab yeah, I think our Auth an AuthPlug code needs an overhaul. 💭
In the meantime do you think we could use https://auth.dwyl.com/ (Fly.io) instead of Heroku for the auth in this App so that there's lower chance of timeout? ⌛

I'm thinking we should build a v2.0 of Auth using mix phx.gen.auth as the starting point
but with Tailwind for the Style/UI etc. ✨
See: dwyl/auth#207 💡
How busy are you next week? 😜
All joking asside. We should catch-up when you're free! 🙏

nelsonic added a commit that referenced this issue Jun 8, 2022
@nelsonic
Copy link
Member Author

nelsonic commented Jun 8, 2022

Could not render "messages.html" for LiveviewChatWeb.MessageView, 
please define a matching clause for render/2 
or define a template at "lib/liveview_chat_web/templates/message/*". 

The following templates were compiled:

* message.html

image

subtle ...

Current instructions:
https://github.com/dwyl/phoenix-liveview-chat-example/tree/4857be73ed4d7e27426357d18621afad4aa1f197#liveview-route-controller-and-template

had:

LiveviewChatWeb.MessageView.render("messages.html", assigns)

Should be:

LiveviewChatWeb.MessageView.render("message.html", assigns)

i.e. the template file is message.html.heex but the render/2 was trying to render messages... (plural!)
This is could either be a useful debugging exercise for the person following along, or a frustrating roadblock.

Anyway, updated it. Working. ✅

image

@nelsonic
Copy link
Member Author

nelsonic commented Jun 8, 2022

Hmm ...

Resolving Hex dependencies...

Failed to use "plug" (versions 1.10.2 to 1.13.6) because
  phoenix (version 1.6.10) requires ~> 1.10
  phoenix_ecto (version 4.4.0) requires ~> 1.9
  phoenix_html (version 3.2.0) requires ~> 1.5
  ping (version 1.1.0) requires ~> 1.12.1
  plug_cowboy (version 2.5.2) requires ~> 1.7


Failed to use "plug" (version 1.12.1) because
  auth_plug (version 1.4.13) requires ~> 1.13.4
  phoenix (version 1.6.10) requires ~> 1.10
  phoenix_ecto (version 4.4.0) requires ~> 1.9
  phoenix_html (version 3.2.0) requires ~> 1.5
  ping (version 1.1.0) requires ~> 1.12.1
  plug_cowboy (version 2.5.2) requires ~> 1.7


Failed to use "telemetry" (versions 0.4.0 and 0.4.1) because
  ecto_sql (version 3.8.3) requires ~> 0.4.0 or ~> 1.0
  phoenix (version 1.6.10) requires ~> 0.4 or ~> 1.0
  phoenix_live_view (version 0.17.10) requires ~> 0.4.2 or ~> 1.0
  plug (versions 1.10.0 and 1.10.1) requires ~> 0.4
  telemetry_metrics (version 0.6.1) requires ~> 0.4 or ~> 1.0


Failed to use "telemetry" (versions 0.4.2 and 0.4.3) because
  ecto_sql (version 3.8.3) requires ~> 0.4.0 or ~> 1.0
  phoenix (version 1.6.10) requires ~> 0.4 or ~> 1.0
  phoenix_live_view (version 0.17.10) requires ~> 0.4.2 or ~> 1.0
  plug (versions 1.10.0 and 1.10.1) requires ~> 0.4
  telemetry_metrics (version 0.6.1) requires ~> 0.4 or ~> 1.0
  telemetry_poller (version 1.0.0) requires ~> 1.0

@nelsonic
Copy link
Member Author

nelsonic commented Jun 8, 2022

Resolving Hex dependencies...
The dependency resolver is taking more than 30 seconds. This typically happens when Hex cannot find a suitable set of dependencies that match your requirements. Here are some suggestions:

  1. Do not delete mix.lock. If you want to update some dependencies, do mix deps.update dep1 dep2 dep3

  2. Tighten up your dependency requirements to the latest version. Instead of {:my_dep, ">= 1.0.0"}, try {:my_dep, "~> 3.6"}

@nelsonic
Copy link
Member Author

nelsonic commented Jun 8, 2022

It's not my internet speed ... 🤷‍♂️

image

@nelsonic
Copy link
Member Author

Took a minor detour to learn Alpine.js: https://github.com/dwyl/learn-alpine.js 🚀
Going to use it and Tailwind CSS for the UI enhancements #4 😉

@nelsonic
Copy link
Member Author

nelsonic commented Jun 16, 2022

This is not a particularly beginner-friendly error message/page: 🙄

attempting to set id attribute to 1, but the DOM ID cannot be set to a number

image

Where does someone new to Phoenix / LiveView even begin to debug this? 🤷‍♂️
It looks like an internal error, not being allowed to set the id of a DOM attribute to 1 ...
This is because we are using the message.id as the id in the UI for uniqueness.

Luckily it's on localhost and I just nuked the DB mix ecto.reset and it worked!

image

But if I were trying to follow along with the tutorial and saw this I would be pretty discouraged!

@nelsonic
Copy link
Member Author

The offending line of code is:

<li id={message.id}>

in: https://github.com/dwyl/phoenix-liveview-chat-example/blame/4857be73ed4d7e27426357d18621afad4aa1f197/README.md#L233

It's correct in the file:

But not in the README.md instructions.

@nelsonic
Copy link
Member Author

Gif at end of section 9:
liveview-chat-demo

@nelsonic
Copy link
Member Author

Going to leave this on Heroku for now. We can deploy to Fly.io in a follow-up PR. 💭

@nelsonic nelsonic added awaiting-review An issue or pull request that needs to be reviewed and removed in-progress An issue or pull request that is being worked on by the assigned person labels Jun 16, 2022
@nelsonic nelsonic assigned SimonLab and unassigned nelsonic Jun 16, 2022
@nelsonic
Copy link
Member Author

4 actions:
image

Just to request review of a task.
We can do much better!

@nelsonic
Copy link
Member Author

PR: #46

nelsonic added a commit that referenced this issue Jun 18, 2022
…ix-liveview-chat-example into expand-instructions-issue-#42
nelsonic added a commit that referenced this issue Jun 18, 2022
SimonLab added a commit that referenced this issue Jun 20, 2022
@nelsonic
Copy link
Member Author

Heroku App not auto-deploying: #47 🙃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-review An issue or pull request that needs to be reviewed chore a tedious but necessary task often paying technical debt enhancement New feature or enhancement of existing functionality good first issue Good for newcomers help wanted If you can help make progress with this issue, please comment! priority-2 Second highest priority, should be worked on as soon as the Priority-1 issues are finished T4h Time Estimate 4 Hours
Projects
None yet
Development

No branches or pull requests

2 participants