search

BibliothecaDAO / eternum

onchain eternal game
https://alpha-eternum.realms.world
MIT License
36 stars 30 forks source link

Fix continue button disabled on acc creation #951

Closed cwastche closed 3 weeks ago

cwastche commented 3 weeks ago

User description

Closes #924

PR Type

Bug fix, Enhancement

Description

Changes walkthrough 📝

Relevant files
Enhancement
Steps.tsx
Fix and enhance naming step in onboarding process               
client/src/ui/modules/onboarding/Steps.tsx
  • Imported MAX_NAME_LENGTH constant.
  • Replaced hardcoded max length value with MAX_NAME_LENGTH.
  • Fixed the condition for enabling the "Continue" button.
  • Minor formatting adjustments.
    		•  +10/-3   

    💡 PR-Agent usage: Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    vercel[bot] commented 3 weeks ago

    The latest updates on your projects. Learn more about Vercel for Git ↗︎

    Name Status Preview Comments Updated (UTC)
    eternum ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 17, 2024 11:05am
    github-actions[bot] commented 3 weeks ago

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review [1-5] 2
    🧪 Relevant tests No
    🔒 Security concerns No
    ⚡ Key issues to review Possible Bug:
    The condition for the "Continue" button enabling has been changed from checking !name to !addressName. Ensure that addressName is the correct variable to check and that it is updated appropriately in all relevant contexts.
    Code Clarity:
    The import statement adds MAX_NAME_LENGTH but it's not clear from the PR description if this constant is defined elsewhere or if it's part of the changes. Clarification or addition in the PR description would be helpful.
    github-actions[bot] commented 3 weeks ago

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Possible bug
    Correct the variable name in the button's disabled property to match the intended state variable ___ **Ensure that the disabled property of the button correctly reflects the intended logic. It
    seems there might be a variable name mismatch (addressName should be inputName if that is
    the intended state variable to check).** [client/src/ui/modules/onboarding/Steps.tsx [271]](https://github.com/BibliothecaDAO/eternum/pull/951/files#diff-0bb7e9e5135bf95ef8376c6e361487312fa9a0226f14fb2e82c63cb57cf5d301R271-R271) ```diff -disabled={!addressName || addressIsMaster} +disabled={!inputName || addressIsMaster} ```
    Suggestion importance[1-10]: 10 Why: This suggestion addresses a potential bug by ensuring the correct variable is used to determine the button's disabled state, which is crucial for the correct functionality of the button.
    		10
    Enhancement
    Ensure the placeholder text dynamically reflects the maximum name length ___ **Replace the hardcoded placeholder text with a dynamic text that reflects the actual
    maximum length allowed for the name input. This will ensure consistency between the
    placeholder and the actual enforced limit.** [client/src/ui/modules/onboarding/Steps.tsx [182]](https://github.com/BibliothecaDAO/eternum/pull/951/files#diff-0bb7e9e5135bf95ef8376c6e361487312fa9a0226f14fb2e82c63cb57cf5d301R182-R182) ```diff -placeholder="Your Name... (Max 31 characters)" +placeholder={`Your Name... (Max ${MAX_NAME_ LENGTH} characters)`} ```
    Suggestion importance[1-10]: 9 Why: This suggestion improves the user experience by ensuring the placeholder text accurately reflects the maximum length allowed for the input, maintaining consistency and reducing confusion.
    		9
    Possible issue
    Add error handling for localStorage retrieval ___ **Consider adding error handling for the localStorage.getItem("burners") call to manage
    cases where the retrieval fails or returns null unexpectedly.** [client/src/ui/modules/onboarding/Steps.tsx [101]](https://github.com/BibliothecaDAO/eternum/pull/951/files#diff-0bb7e9e5135bf95ef8376c6e361487312fa9a0226f14fb2e82c63cb57cf5d301R101-R101) ```diff -const burners = localStorage.getItem("burners"); +const burners = localStorage.getItem("burners") || "default_value"; ```
    Suggestion importance[1-10]: 7 Why: Adding error handling for the localStorage retrieval can prevent potential issues if the retrieval fails or returns null, improving the robustness of the code.
    		7
    Maintainability
    Remove unnecessary empty line to enhance code cleanliness ___ **To improve code readability and maintainability, consider removing the unnecessary empty
    line added in this PR.** [client/src/ui/modules/onboarding/Steps.tsx [99]](https://github.com/BibliothecaDAO/eternum/pull/951/files#diff-0bb7e9e5135bf95ef8376c6e361487312fa9a0226f14fb2e82c63cb57cf5d301R99-R99) ```diff -(empty line) +(remove the empty line) ```
    Suggestion importance[1-10]: 5 Why: While removing the unnecessary empty line improves code readability and maintainability, it is a minor issue and does not significantly impact the functionality of the code.
    		5