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

refactor(DashboardCreationController): update placeholder #1109

Open
wants to merge 2 commits into
base: staging
Choose a base branch
from

Conversation

benji-bitfly
Copy link
Contributor

See: BEDS-570

Copy link

cloudflare-workers-and-pages bot commented Nov 8, 2024

Deploying beaconchain with  Cloudflare Pages  Cloudflare Pages

Latest commit: f642e06
Status: ✅  Deploy successful!
Preview URL: https://165a35d3.beaconchain.pages.dev
Branch Preview URL: https://beds-570-cookie-placeholder.beaconchain.pages.dev

View logs

@benji-bitfly benji-bitfly force-pushed the BEDS-570/cookie-placeholder-dashboardcreation branch 4 times, most recently from d967a64 to 7bc10fc Compare November 8, 2024 10:18
@benji-bitfly benji-bitfly changed the title refactor(DashboardCreationController): remove placeholder refactor(DashboardCreationController): update placeholder Nov 8, 2024
@benji-bitfly benji-bitfly force-pushed the BEDS-570/cookie-placeholder-dashboardcreation branch 4 times, most recently from 48e3e50 to 1912cb3 Compare November 11, 2024 14:03
@benji-bitfly benji-bitfly marked this pull request as ready for review November 11, 2024 14:06
@marcel-bitfly
Copy link
Contributor

np: commit msg is kinda misleading atm

…oard creation`

When `user` starts the dashboard creation flow the user will have display the `input`field and start `user journey`.

See: BEDS-570
@benji-bitfly benji-bitfly force-pushed the BEDS-570/cookie-placeholder-dashboardcreation branch from 1912cb3 to f642e06 Compare November 11, 2024 14:34
@@ -78,7 +78,7 @@ function show(
}
network.value = forcedNetwork || currentNetwork.value
state.value = 'type'
name.value = isLoggedIn.value ? '' : 'cookie'
name.value = isLoggedIn.value ? '' : ''
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not entirely sure on my own what this line does...which means that the whole feature needs refactoring...but plz never do things like this, because this line no is max-confusing

Suggested change
name.value = isLoggedIn.value ? '' : ''

(would exactly be the same)

@@ -117,7 +117,7 @@
"title": "Add a new dashboard",
"type": {
"accounts": "Accounts",
"placeholder": "Name",
"placeholder": "Enter dashboard name",
Copy link
Contributor

Choose a reason for hiding this comment

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

nice that you improve things along the way 💪.

But we are using placeholder as input labels.

See: https://www.deque.com/blog/accessible-forms-the-problem-with-placeholders/

Placeholder value should be an example of the things a user should input.
Maybe you come up with a different idea.

@marcel-bitfly
Copy link
Contributor

np: commit msgs: plz try to describe your changes as simple as possible, also double check your grammar to prevent confusion

refactor(DashboardCreationController): remove cookie for dashboard name

In the dashboard creation process the input field field for the dashboard name
was prefilled with cookie.

See: BEDS-570


feat(DashboardCreationController): update placeholder for dashboard name

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.

2 participants