Migrate Home and OS data page to components #231
Conversation
vegg89
changed the title
|
Hi @vegg89! Thank you so much for the PR. I am a bit behind on my open source duties but I haven't forgot about this. It is sitting on my inbox. :) I hope to get this reviewed by next week the latest! |
| <%= live_component @socket, ColorBarComponent, dom_id: "cpu-#{usage.dom_sub_id}", data: usage.data, title: usage.title, csp_nonces: @csp_nonces %> | ||
| </div> | ||
| <% end %> | ||
| <%= live_component @socket, ColorBarLegendComponent, data: @total_data, formatter: @total_formatter %> |
There was a problem hiding this comment.
It would be nice to remove this component and instead make if a function or just inline the code directly here. :) The same for ColorBarComponent above.
There was a problem hiding this comment.
I removed the components and added the code directly to the render.
| |> PageColumnsComponent.normalize_params() | ||
|
|
||
| {PageColumnsComponent, assigns} | ||
| end |
There was a problem hiding this comment.
There is a bit of inconsistency between page_columns and row. Is page_columns prefixed with page_ because it can only be used at the root? If so, should we document that? Also, we do page_columns(columns: [...]) and row(components: [...]). Should it be called components for both?
There was a problem hiding this comment.
Initially I decide to use page_columns to avoid confusion with columns/0 function used in tables. I change the latter to table_columns/0 and rename page_columns to columns
| * `:usages` - Required. A list of `Map` with the following keys: | ||
| * `:data` - A list of tuples with 4 elements with the following data: | ||
| `{usage_name, usage_percent, color, hint}` | ||
| * `:sub_dom_id` - Required. Usage identifier. |
There was a problem hiding this comment.
I did the correction!
|
Hi @vegg89, sorry for the delay, I have finally looked into it. This looks great! I have dropped some comments but they are minor. :) Let me know if you have any questions, review should be quicker from now on! |
|
Hi @josevalim, thank you for your review, I did the corrections that you mention, please let me know if you notice any other detail! 🤓 |
|
💚 💙 💜 💛 ❤️ |
|
Thank you!! 😃🤟🏽 |
Hi everyone,
I worked on the issue #216 and migrate the Home and OS Data pages to components.
In order to achieve this, I created 6 components and add them to the PageBuilder module:
columnsparams