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

[PLA-1701] UI improvements #88

Merged
merged 10 commits into from
Apr 4, 2024
Merged

Conversation

zlayine
Copy link
Contributor

@zlayine zlayine commented Mar 29, 2024

Type

enhancement, documentation


Description

  • Introduced 'simple' and 'advanced' modes for creating tokens and collections, enhancing user experience with more options.
  • Added new fields for token and collection creation, including 'Image URL', 'Name', 'Description', and token type selection.
  • Implemented dynamic form validation across various forms to ensure data integrity.
  • Enhanced UI with documentation links, tooltips, and improved error handling for a better user experience.
  • Refactored several components for simplified templates and better readability.
  • Implemented a new RichTextEditor component utilizing EasyMDE for enhanced text editing capabilities.

Changes walkthrough

Relevant files
Enhancement
13 files
BatchMintForm.vue
Refactor BatchMintForm for Dynamic Validation and Simplify Template

resources/js/components/batch/forms/BatchMintForm.vue

  • Added formValidation computed property to dynamically select
    validation schema based on mintType.
  • Replaced createValidation with formValidation in component.
  • Simplified template structure for better readability and maintenance.
  • Added v-if conditionals for advanced options based on store state.
  • +257/-241
    CreateToken.vue
    Enhance CreateToken Page with New Fields and Mode Switch 

    resources/js/components/pages/create/CreateToken.vue

  • Introduced a mode switch between 'simple' and 'advanced' for creating
    tokens.
  • Added new input fields for token creation, including 'Image URL',
    'Name', 'Description', and token type selection.
  • Implemented dynamic form validation based on selected token type.
  • Enhanced UI with documentation links and improved layout.
  • +195/-132
    CreateCollection.vue
    Add New Fields and Mode Toggle to CreateCollection Page   

    resources/js/components/pages/create/CreateCollection.vue

  • Added 'simple' and 'advanced' mode toggle for creating collections.
  • Introduced new fields for collection creation, including 'Image URL',
    'Banner URL', 'Name', and 'Description'.
  • Implemented dynamic form validation and enhanced UI with tooltips and
    documentation links.
  • +200/-72
    CreateFuelTank.vue
    Simplify CreateFuelTank Template and Add Documentation Link

    resources/js/components/pages/create/CreateFuelTank.vue

  • Simplified template structure and improved readability.
  • Added documentation link for creating fuel tanks.
  • Implemented dynamic form validation for fuel tank creation.
  • +130/-124
    DispatchRuleForm.vue
    Refactor DispatchRuleForm for Simplified Template and UI Enhancement

    resources/js/components/fueltank/DispatchRuleForm.vue

  • Simplified template structure for dispatch rule form.
  • Enhanced UI with better layout and readability.
  • +155/-158
    BatchTransferForm.vue
    Refactor BatchTransferForm with Dynamic Validation and Simplified
    Template

    resources/js/components/batch/forms/BatchTransferForm.vue

  • Refactored to use dynamic validation schema based on transfer type.
  • Simplified template structure and added conditional rendering for
    advanced options.
  • +101/-127
    BatchTransfer.vue
    Enhance BatchTransfer Component with Tooltips and Documentation Links

    resources/js/components/batch/BatchTransfer.vue

  • Added tooltips for better user guidance.
  • Enhanced UI with documentation links and improved layout.
  • +63/-59 
    BatchMint.vue
    Enhance BatchMint Component with Tooltips and Documentation Links

    resources/js/components/batch/BatchMint.vue

  • Added tooltips for better user guidance.
  • Enhanced UI with documentation links and improved layout.
  • +57/-53 
    BatchSetAttribute.vue
    Enhance BatchSetAttribute Component with Tooltips and Documentation
    Links

    resources/js/components/batch/BatchSetAttribute.vue

  • Added tooltips for better user guidance.
  • Enhanced UI with documentation links and improved layout.
  • +45/-45 
    CreateBeam.vue
    Add Documentation Link and Implement Dynamic Validation in CreateBeam

    resources/js/components/pages/create/CreateBeam.vue

  • Added documentation link for creating beams.
  • Implemented dynamic form validation for beam creation.
  • +30/-25 
    RichTextEditor.vue
    Implement RichTextEditor Component with EasyMDE                   

    resources/js/components/RichTextEditor.vue

  • Introduced a new RichTextEditor component for enhanced text editing.
  • Utilized EasyMDE for markdown editing and added custom styling.
  • +154/-0 
    FormInput.vue
    Enhance FormInput Component with Better Error Handling     

    resources/js/components/FormInput.vue

    • Improved error message display and added conditional rendering.
    +8/-5     
    FormSelect.vue
    Enhance FormSelect Component with Better Error Handling   

    resources/js/components/FormSelect.vue

    • Improved error message display and added conditional rendering.
    +4/-3     

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

    @zlayine zlayine self-assigned this Mar 29, 2024
    @github-actions github-actions bot added documentation Improvements or additions to documentation enhancement New feature or request labels Mar 29, 2024
    Copy link

    PR Description updated to latest commit (db7414b)

    Copy link

    PR Review

    ⏱️ Estimated effort to review [1-5]

    4, due to the extensive changes across multiple files, including the introduction of new components and modifications to existing ones. The changes impact both the functionality and UI, requiring a thorough review to ensure consistency, performance, and adherence to best practices.

    🧪 Relevant tests

    No

    🔍 Possible issues

    Possible Bug: The use of v-if="useAppStore().advanced" directly in templates without a computed property or method might cause reactivity issues or errors if useAppStore is not returning a reactive object.

    Performance Concern: The extensive use of v-for loops and dynamic components in forms, especially in 'BatchMint.vue' and 'CreateBeam.vue', could lead to performance issues with a large number of items.

    Code Duplication: Similar code patterns are used across different components for form handling and validation. Consider abstracting these patterns into reusable functions or components to reduce duplication and improve maintainability.

    🔒 Security concerns

    No

    Code feedback:
    relevant fileresources/js/components/batch/forms/BatchMintForm.vue
    suggestion      

    Consider using a computed property for useAppStore().advanced to ensure reactivity and prevent potential errors. [medium]

    relevant linev-if="useAppStore().advanced"

    relevant fileresources/js/components/pages/create/CreateBeam.vue
    suggestion      

    For improved performance with large lists, consider using a virtual list or pagination for rendering the assets in the beam creation form. [medium]

    relevant linev-for="(item, idx) in tokens"

    relevant fileresources/js/components/batch/BatchSetAttribute.vue
    suggestion      

    Abstract the repeated form input patterns into a reusable component to reduce code duplication and improve maintainability. [medium]

    relevant line

    relevant fileresources/js/components/beam/ClaimableTokenForm.vue
    suggestion      

    Use v-model.trim on text inputs to automatically trim whitespace, improving data quality and user experience. [medium]

    relevant linev-model="inputs.key"


    ✨ Review tool usage guide:

    Overview:
    The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

    The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

    • When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:
    /review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...
    
    [pr_reviewer]
    some_config1=...
    some_config2=...
    

    See the review usage page for a comprehensive guide on using this tool.

    Copy link

    PR Code Suggestions

    CategorySuggestions                                                                                                                                                       
    Enhancement
    Add a key attribute to list items for efficient rendering.

    Consider adding a key attribute to the element inside the component to ensure that Vue
    can track each node's identity and efficiently re-render lists.

    resources/js/components/Address.vue [4-6]

    -<span class="break-words">
    +<span class="break-words" :key="`address-${someUniqueIdentifier}`">
         {{ displayAddress }}
     </span>
     
    Dynamically change input border color based on validation state.

    To improve form validation feedback, consider dynamically applying a red border color to
    the input field when errorMessage is not empty, enhancing visual cues for error states.

    resources/js/components/FormInput.vue [40]

    -'block flex-1 flex-shrink-0 border-0 text-gray-900 ring-1 ring-inset ring-gray-300 placeholder:text-gray-400 focus:ring-2 focus:ring-inset focus:ring-primary text-sm leading-6 overflow-hidden transition-all disabled:bg-gray-100 outline-none',
    +'block flex-1 flex-shrink-0 border-0 text-gray-900 ring-1 ring-inset ' + (errorMessage ? 'ring-red-500' : 'ring-gray-300') + ' placeholder:text-gray-400 focus:ring-2 focus:ring-inset focus:ring-primary text-sm leading-6 overflow-hidden transition-all disabled:bg-gray-100 outline-none',
     
    Sync EasyMDE content with Vue model value using a watcher.

    To ensure the EasyMDE editor properly initializes with the current modelValue, consider
    adding a watcher on localModelValue that updates the EasyMDE content whenever the model
    value changes.

    resources/js/components/RichTextEditor.vue [84-101]

     const initMDE = () => {
         const easyMDE = new EasyMDE({
             ...
         });
    +    watch(localModelValue, (newValue) => {
    +        if (easyMDE.value() !== newValue) {
    +            easyMDE.value(newValue);
    +        }
    +    });
     };
     
    Add a v-for directive with a key attribute to the component.

    Consider using a v-for directive with a key attribute for the component to ensure that
    Vue can track each component instance and maintain state accordingly. This is particularly
    useful if you plan to have multiple tooltips with dynamic content.

    resources/js/components/batch/BatchMint.vue [9-13]

     <Tooltip
    -    text="Use this method to batch together several mints into one transaction. You can mix and
    -    match Create Token and Mint Token params, as well as use the continueOnFailure flag to
    -    skip mints which fail on chain so they can be fixed later."
    +    v-for="(tooltipText, index) in tooltips"
    +    :key="`tooltip-${index}`"
    +    :text="tooltipText"
     >
     
    Implement a custom validation rule for the idempotencyKey.

    To improve form validation and user experience, consider implementing a custom validation
    rule for the idempotencyKey that checks for UUID format, ensuring that users input a
    correctly formatted UUID.

    resources/js/components/batch/BatchMint.vue [29]

     v-model="idempotencyKey"
    +:rules="{ uuid: value => /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i.test(value) || 'Please enter a valid UUID' }"
     
    Automatically trim whitespace from user inputs.

    Consider using v-model.trim on FormInput and TokenIdInput components to automatically trim
    whitespace from the user input. This can prevent errors or inconsistencies due to
    accidental leading or trailing spaces.

    resources/js/components/batch/forms/BatchTransferForm.vue [14-20]

     <FormInput
    -    v-model="account"
    +    v-model.trim="account"
         name="account"
         label="Account"
         description="The recipient account of the token."
         required
     />
     
    Add validation error messages to input fields.

    To improve form validation feedback, consider adding a visual indicator or message near
    each input field to display validation errors. This can enhance user experience by making
    it easier to identify and correct input errors.

    resources/js/components/batch/forms/BatchTransferForm.vue [14-20]

     <FormInput
         v-model="account"
         name="account"
         label="Account"
         description="The recipient account of the token."
    +    :error-messages="formErrors['account']"
         required
     />
     
    Add a loading state to the form submission button.

    For better user experience and to avoid potential layout shifts, consider adding a loading
    state to the form submission button. This can inform users that their action is being
    processed and prevent multiple submissions.

    resources/js/components/beam/ClaimableTokenForm.vue [35]

    -<FormSelect
    +<button :disabled="isSubmitting" class="btn-primary">
    +    <span v-if="isSubmitting">Submitting...</span>
    +    <span v-else>Submit</span>
    +</button>
     
    Improve accessibility by adding aria-label to inputs.

    For better accessibility and user experience, consider adding aria-label attributes to
    inputs and select elements. This helps users who rely on screen readers to understand the
    purpose of each input field.

    resources/js/components/fueltank/DispatchRuleForm.vue [30-34]

     <input
         v-model="inputs.caller"
         :dusk="`input__caller-${index + 1}`"
         type="text"
    +    aria-label="Caller"
         class="block w-full rounded-md border-0 py-1.5 text-gray-900 shadow-sm ring-1 ring-inset ring-gray-300 placeholder:text-gray-400 focus:ring-2 focus:ring-inset focus:ring-primary sm:text-sm sm:leading-6"
     />
     
    Best practice
    Use semantic HTML tags for better accessibility.

    For better accessibility and semantic HTML, consider using the

    tag for the primary
    content of the instead of a
    .

    resources/js/components/App.vue [7]

    -<div class="flex flex-col w-full overflow-hidden">
    +<main class="flex flex-col w-full overflow-hidden">
     
    Use a computed property for collectionIds options.

    It's recommended to use a computed property for collectionIds options to ensure they are
    reactive to changes, especially if collectionIds are fetched asynchronously or can change
    based on user interaction.

    resources/js/components/batch/BatchSetAttribute.vue [25]

    -:options="collectionIds"
    +:options="computedCollectionIds"
     
    Ensure reactivity for the dynamic list of transfers.

    To ensure the dynamic list of transfers is reactive and updates correctly in the Vue
    component, use v-model with a deep clone of item.values in , or consider using a method to
    update the item values to maintain reactivity.

    resources/js/components/batch/BatchTransfer.vue [70]

    -<BatchTransferForm v-model="item.values" @validation="setValidation(idx, $event)" />
    +<BatchTransferForm :model-value="item.values" @update:model-value="newValue => updateItemValues(idx, newValue)" @validation="setValidation(idx, $event)" />
     
    Use Vue's nextTick for DOM update related operations.

    For the setTimeout used in the validation and model update, consider using Vue's nextTick
    function instead. This ensures that the code inside setTimeout is executed after the next
    DOM update cycle, which is more reliable and Vue idiomatic for this use case.

    resources/js/components/batch/forms/BatchTransferForm.vue [213-216]

    -setTimeout(() => {
    +this.$nextTick(() => {
         emit('validation', validForm.value);
         emit('update:modelValue', hasChanged.value);
    -}, 50);
    +});
     
    Provide default values for data properties to enhance stability.

    To ensure a more consistent and predictable user experience, consider providing a default
    value for whitelistedCallers and whitelistedCollections in the component's data. This
    helps prevent potential issues when these properties are undefined.

    resources/js/components/fueltank/DispatchRuleForm.vue [9-16]

    -<FormList
    -    v-model="whitelistedCallers"
    -    @add="addCaller"
    -    @remove="removeCaller"
    -    flex
    -    add-text="Add Caller"
    -    dusk-id="caller"
    ->
    +data() {
    +    return {
    +        whitelistedCallers: [], // Default value as an empty array
    +        whitelistedCollections: [], // Default value as an empty array
    +    };
    +}
     
    Use await with setTimeout to handle asynchronous operations correctly.

    To avoid potential race conditions and ensure the form validation and state updates are
    handled correctly, consider using await with the setTimeout function inside the async
    function.

    resources/js/components/fueltank/DispatchRuleForm.vue [298-301]

    -setTimeout(() => {
    -    emit('validation', validForm.value);
    -    emit('update:modelValue', hasChanged.value);
    -}, 50);
    +await new Promise(resolve => setTimeout(resolve, 50));
    +emit('validation', validForm.value);
    +emit('update:modelValue', hasChanged.value);
     
    Accessibility
    Add aria-label for better accessibility and user feedback.

    To improve user feedback, consider adding an aria-label attribute to the

    that wraps the
    ClipboardIcon to indicate the action that will be performed when clicked.

    resources/js/components/CopyTextIcon.vue [2]

    -<div class="cursor-pointer" @click="copyText">
    +<div class="cursor-pointer" @click="copyText" aria-label="Copy to clipboard">
     
    Add aria-label attributes to icons for better accessibility.

    For better accessibility, consider adding aria-label attributes to icons, especially when
    they serve as interactive elements (buttons, links, etc.), to provide screen reader users
    with context about the action or information the icon represents.

    resources/js/components/batch/BatchTransfer.vue [68]

    -<XMarkIcon class="h-5 w-5 cursor-pointer" @click.prevent="removeItem(idx)" />
    +<XMarkIcon class="h-5 w-5 cursor-pointer" @click.prevent="removeItem(idx)" aria-label="Remove item" />
     
    Improve accessibility by adding aria-label to inputs.

    To enhance accessibility and provide better context to users, consider adding aria-label
    attributes to inputs that only have placeholders. This helps screen readers convey the
    purpose of the input fields more clearly.

    resources/js/components/beam/ClaimableTokenForm.vue [13-20]

     <FormChipsInput
         v-model="tokenIds"
         name="tokenIds"
         label="Token IDs"
         tooltip="The token chain IDs available to claim."
         placeholder="1, 2 or 10..20 (Integers range)"
         :regex-function="testInteger"
    +    aria-label="Token IDs"
     />
     
    Performance
    Conditionally render components based on state to improve performance.

    Consider using v-if to conditionally render the FormInput components for "Require Token"
    based on the isModal state. This can improve the performance by avoiding the rendering of
    unnecessary elements when the modal is not active.

    resources/js/components/fueltank/DispatchRuleForm.vue [47-55]

    -<div :class="`grid grid-cols-${isModal ? '1' : '2'} gap-4`">
    +<div v-if="!isModal" class="grid grid-cols-2 gap-4">
         <FormInput
             class="col-span-1"
             v-model="collectionId"
             name="collectionId"
             type="number"
             placeholder="Collection ID"
         />
         <TokenIdInput class="col-span-1" v-model="tokenId" placeholder="Token ID" />
     </div>
     
    Maintainability
    Extract templates into separate components for better maintainability.

    To enhance code readability and maintainability, consider extracting the templates used
    within FormList components into separate Vue components. This modular approach can make
    the templates easier to manage, especially as they grow in complexity.

    resources/js/components/fueltank/DispatchRuleForm.vue [17-26]

    -<template #headers>
    -    <div class="flex-1">
    -        <label class="block text-sm font-medium leading-6 text-gray-900">
    -            Whitelisted Callers
    -        </label>
    -        <p class="mt-1 text-sm text-gray-500">
    -            The wallet accounts that are allowed to use the fuel tank.
    -        </p>
    -    </div>
    -    <div class="w-5"></div>
    -</template>
    +<WhitelistedCallersHeader />
     

    ✨ Improve tool usage guide:

    Overview:
    The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.

    • When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:
    /improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...
    
    [pr_code_suggestions]
    some_config1=...
    some_config2=...
    

    See the improve usage page for a comprehensive guide on using this tool.

    @zlayine zlayine force-pushed the feature/pla-1701/ui-improvements branch from e7eba38 to b1e0a95 Compare April 1, 2024 15:12
    @zlayine zlayine force-pushed the feature/pla-1701/ui-improvements branch from b1e0a95 to 1307a06 Compare April 1, 2024 15:15
    @zlayine zlayine marked this pull request as ready for review April 1, 2024 15:15
    @zlayine zlayine merged commit 0be2a0b into master Apr 4, 2024
    3 checks passed
    @zlayine zlayine deleted the feature/pla-1701/ui-improvements branch April 4, 2024 09:56
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    documentation Improvements or additions to documentation enhancement New feature or request Review effort [1-5]: 4
    Development

    Successfully merging this pull request may close these issues.

    2 participants