Dashboard button #25

Merged
meraghon merged 8 commits from feat/dashboard-button into main 2025-12-05 13:35:07 +00:00
meraghon commented 2025-11-24 00:13:27 +00:00 (Migrated from github.com)

Created a dashboard button that is used to redirect user to a specified page. Has a title, description, and option for an icon.

Temporary test page for testing.

Created a dashboard button that is used to redirect user to a specified page. Has a title, description, and option for an icon. Temporary test page for testing.
BobdaFett (Migrated from github.com) requested changes 2025-11-27 21:29:58 +00:00
BobdaFett (Migrated from github.com) left a comment

Refer to my comments.

Refer to my comments.
@@ -0,0 +1,61 @@
use leptos::prelude::*;
BobdaFett (Migrated from github.com) commented 2025-11-27 21:22:23 +00:00

These should not be Optional values. When we use #[prop(optional), it just means that we either have a value, or the default value. For something like this (a String) that would mean we either have a specific value passed in or just the empty string.

These should not be Optional values. When we use `#[prop(optional)`, it just means that we either have a value, or the default value. For something like this (a String) that would mean we either have a specific value passed in or just the empty string.
BobdaFett (Migrated from github.com) commented 2025-11-27 21:23:30 +00:00

Setting up the #[prop] the right way above should mean that you just print the string value here, no mapping needed.

Setting up the `#[prop]` the right way above should mean that you just print the string value here, no mapping needed.
BobdaFett (Migrated from github.com) commented 2025-11-27 21:26:03 +00:00

Depending on your desired behavior, leaving this as an Option<String> might be fine. What I'd like to know is the mindset behind it - if this is a None value, how are you handling it? Should this even be optional at all? Why?

I'd recommend documenting this properly (refer to Leptos documentation for how to add doc comments to components).

Depending on your desired behavior, leaving this as an `Option<String>` might be fine. What I'd like to know is the mindset behind it - if this is a `None` value, how are you handling it? Should this even be optional at all? Why? I'd recommend documenting this properly (refer to Leptos documentation for how to add doc comments to components).
@@ -0,0 +19,4 @@
/// </div>
/// ```
#[component]
pub fn DashboardButton(
BobdaFett (Migrated from github.com) commented 2025-11-27 21:28:40 +00:00

This whole component needs documentation - at least give a title to the component, a brief description, explain the props, and a usage example. This is included with other components that have been created, specifically the selection lists (ChipsList, CheckboxList, etc.).

This whole component needs documentation - at least give a title to the component, a brief description, explain the props, and a usage example. This is included with other components that have been created, specifically the selection lists (`ChipsList`, `CheckboxList`, etc.).
BobdaFett (Migrated from github.com) requested changes 2025-12-05 13:08:37 +00:00
@@ -0,0 +1,61 @@
use leptos::prelude::*;
BobdaFett (Migrated from github.com) commented 2025-12-05 13:07:30 +00:00

These props should also have comments. They just have the /// comment one line above.

These props should also have comments. They just have the `/// comment` one line above.
BobdaFett (Migrated from github.com) commented 2025-12-05 13:08:03 +00:00

There should always be a space after the # in headers.

There should always be a space after the `#` in headers.
BobdaFett (Migrated from github.com) approved these changes 2025-12-05 13:34:48 +00:00
Sign in to join this conversation.