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

[#2024-03] Improve address selection UI #84

Merged
merged 17 commits into from
Mar 24, 2024

Conversation

delano
Copy link
Contributor

@delano delano commented Mar 24, 2024

User description

  • Make address a required field
  • Replace custom location fields with Google Places autocomplete
  • Add building type radiogroup
  • Set autocomplete to strict bounds within Canada
  • Request only necessary Google Maps libraries

Type

enhancement, documentation, other


Description

  • Integrated Google Maps Places API for address autocomplete in the new request form.
  • Added a new Vue component for location-based search functionality.
  • Example integration of Google Maps with population circles visualization.
  • Updated TypeScript definitions to support Google Maps integration.
  • Removed Vueform Builder from project dependencies and configurations.
  • Introduced Changie for changelog management and added initial changelog documentation.
  • Configured custom commands and ignore patterns for Continuer.

Changes walkthrough

Relevant files
Enhancement
6 files
FoodDeliveryForm.vue
Refactor Food Delivery Form with Google Places Integration

apps/ui/components/requests/FoodDeliveryForm.vue

  • Added googleMapsIsReady prop to handle Google Maps API readiness.
  • Changed form size from "md" to "lg" and updated console log messages
    for clarity.
  • Removed default address fields and replaced with a single
    interactive_address field using Google Places autocomplete.
  • Simplified and reorganized form schema, including moving static
    elements to the bottom.
  • +75/-108
    new.vue
    Integrate Google Maps Places API in New Request Form         

    apps/ui/pages/requests/new.vue

  • Introduced Google Maps Places API integration for address
    autocomplete.
  • Added dashboard layout and authentication checks.
  • Updated form state to include interactive_address and changed
    preferred contact method to "Email".
  • +89/-25 
    loco.vue
    Add New Location-Based Search Form Component                         

    apps/ui/pages/requests/loco.vue

  • New Vue component for location-based form with date, guest count, and
    search functionality.
  • +185/-0 
    vue3.vue
    Example Google Maps Integration with Population Circles   

    apps/ui/pages/requests/vue3.vue

  • Example Google Maps integration with circles representing city
    populations.
  • +70/-0   
    app.vue
    Prepare App Component for Google Maps Integration               

    apps/ui/app.vue

  • Prepared for Google Maps Places API integration.
  • Added console log on component mount.
  • +19/-1   
    index.d.ts
    Update TypeScript Definitions for Google Maps Integration

    apps/ui/types/index.d.ts

  • Added global declaration for Google Maps and updated
    FoodDeliveryFormState interface.
  • +14/-2   
    Formatting
    1 files
    dashboard.vue
    Format Adjustment in Dashboard Layout                                       

    apps/ui/layouts/dashboard.vue

    • Minor formatting adjustment.
    +2/-0     
    Configuration changes
    6 files
    tailwind.config.ts
    Update Tailwind Configuration                                                       

    apps/ui/tailwind.config.ts

    • Removed Vueform Builder from Tailwind configuration.
    +3/-3     
    vueform.config.ts
    Comment Out Vueform Builder Plugin Import                               

    apps/ui/vueform.config.ts

    • Commented out Vueform Builder plugin import.
    +9/-2     
    nuxt.config.ts
    Update Nuxt Configuration                                                               

    apps/ui/nuxt.config.ts

    • Removed Vueform Builder Nuxt module.
    +5/-1     
    .changie.yaml
    Add Changie Configuration for Changelog Management             

    .changie.yaml

    • Introduced Changie configuration for changelog management.
    +26/-0   
    .continuerc.json
    Configure Custom Command for Continuer                                     

    .continuerc.json

    • Added custom command configuration for Continuer.
    +9/-0     
    .continueignore
    Define Ignore Patterns for Continuer                                         

    .continueignore

    • Defined ignore patterns for Continuer.
    +16/-0   
    Dependencies
    1 files
    package.json
    Update Package Dependencies                                                           

    apps/ui/package.json

  • Removed Vueform Builder Nuxt module and added vue3-google-map package.
  • Added @types/google.maps for TypeScript support.
  • +3/-5     
    Documentation
    2 files
    CHANGELOG.md
    Initialize Changelog File                                                               

    CHANGELOG.md

    • Initial changelog file generated by Changie.
    +9/-0     
    header.tpl.md
    Add Changelog Header Template                                                       

    .changes/header.tpl.md

    • Template for changelog header.
    +6/-0     

    delano added 14 commits March 21, 2024 15:12
    Builder is working great with the most recent config updates. Removing
    for now to simplify google maps / Places API debugging.
    Signed-off-by: delano <delano@cpan.org>
    Signed-off-by: delano <delano@cpan.org>
    e.g. node_modules, .nuxt and other total rabbit hole
    
    Signed-off-by: delano <delano@cpan.org>
    Signed-off-by: delano <delano@cpan.org>
    See https://localhost/requests/vue3.
    This time window.google is loaded implicitly by vue3-googl-maps.
    See: https://localhost/requests/new
    
    Note: With the provider_location in schema commented out.
    Looking into this possible solution for Autocomplete error:
    https://discord.com/channels/787237947635793940/1192044403506814986
    The `Uncaught (in promise) TypeError: Cannot read properties of undefined (reading 'Autocomplete')` errors still apear but the LocationElement is suggestion as you type now!
    
    (At least in the stripped down, hard-coded example in new.vue itself.)
    
    ![Screenshot 2024-03-23 at 13 44 21](https://github.com/AnimalFoodBank/afb-requests/assets/1206/91ec7201-bd3c-4609-a5fa-cbc823fbd6e8)
    * Removes autocomplete and places related code no longer needed
    * Switches to simpler text input for address instead
    * Sets googleMapsIsReady ref when maps API loads
    * Passes through googleMapsIsReady prop to form
    * Removes unused vueform examples
    
    The removed code and components were initially added while evaluating the Google Maps API but were not being used in the current flow. By cleaning these up, the page is simpler and avoids unnecessary API calls.
    
    The text input replaces the autocomplete LocationElement input from Vueform for now. The googleMapsIsReady ref is now set once the maps API Promise resolves. This is passed as a prop to FoodDeliveryForm to enable reintroducing maps functionality later on.
    
    Overall this commit refactors the code to be more purposeful and optimized for the current usage, while still allowing maps features to be rebuilt in the future.
    - Make address a required field
    - Replace custom location fields with Google Places autocomplete
    - Add building type radiogroup
    - Set autocomplete to strict bounds within Canada
    - Request only necessary Google Maps libraries
    Copy link

    joshua-keaton bot commented Mar 24, 2024

    PR Description updated to latest commit (9f85270)

    Copy link

    joshua-keaton bot commented Mar 24, 2024

    PR Review

    ⏱️ Estimated effort to review [1-5]

    4, due to the significant amount of changes across multiple files, including the integration of Google Maps API, removal of Vueform Builder, and adjustments in Vue components. The complexity of integrating third-party APIs and ensuring compatibility with existing systems increases the review effort. Additionally, the introduction of new Vue components and the removal of dependencies require careful consideration of potential impacts on the application's functionality and user experience.

    🧪 Relevant tests

    No

    🔍 Possible issues

    Hardcoded API Key: The Google Maps API key is hardcoded in multiple files. This poses a security risk and makes it difficult to manage the API key across different environments.

    Lack of Error Handling: There is no visible error handling for the Google Maps API integration. This could lead to a poor user experience if the API fails to load or if there are issues with the API requests.

    Potential Performance Concerns: The integration of Google Maps API and the changes in Vue components could impact the application's performance, especially on pages with map functionalities. It's important to monitor the performance and optimize as necessary.

    🔒 Security concerns

    Hardcoded API Key: The Google Maps API key is hardcoded in multiple files (new.vue, vue3.vue, app.vue). This exposes the key to the public and poses a security risk. It's recommended to use environment variables to manage API keys securely.


    import { Loader } from "@googlemaps/js-api-loader";

    const googleAPIKey = 'AIzaSyC_UvqrTnimc1Pc7LDYCqdqUiGMMUgMCWg'
    Copy link

    Choose a reason for hiding this comment

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

    Consider using environment variables for the Google Maps API key instead of hardcoding it. This enhances security and makes it easier to manage the key across different environments. [important]

    Copy link
    Contributor Author

    Choose a reason for hiding this comment

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

    I want to check if there's a way to sign front-end Google API requests via the backend but it's low-prio. This is just how these keys are used. The security is in how they are restricted from the Credentials section of the GCP console.


    const loader = new Loader({
    apiKey: googleAPIKey,
    version: 'beta',
    Copy link

    Choose a reason for hiding this comment

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

    Optimize the Google Maps API usage by requesting only the necessary libraries and using asynchronous loading to improve page load times and overall performance. [medium]

    Copy link
    Contributor Author

    Choose a reason for hiding this comment

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

    You'll notice right below, we're only requesting the Places API : ]

    libraries: ['places'],
    });


    Copy link

    Choose a reason for hiding this comment

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

    Add a cleanup function to properly remove event listeners or other resources associated with the Google Maps API when the component is destroyed. This prevents potential memory leaks. [medium]

    Copy link
    Contributor Author

    Choose a reason for hiding this comment

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

    Great idea. Thank you!

    Copy link

    joshua-keaton bot commented Mar 24, 2024

    PR Agent Walkthrough

    🤖 Welcome to the PR Agent, an AI-powered tool for automated pull request analysis, feedback, suggestions and more.

    Here is a list of tools you can use to interact with the PR Agent:

    ToolDescriptionInvoke Interactively 💎

    DESCRIBE

    Generates PR description - title, type, summary, code walkthrough and labels
    • Run

    REVIEW

    Adjustable feedback about the PR, possible issues, security concerns, review effort and more
    • Run

    IMPROVE

    Code suggestions for improving the PR.
    • Run

    ANALYZE 💎

    Identifies code components that changed in the PR, and enables to interactively generate tests, docs, and code suggestions for each component.
    • Run

    UPDATE CHANGELOG

    Automatically updates the changelog.
    • Run

    ADD DOCUMENTATION 💎

    Generates documentation to methods/functions/classes that changed in the PR.
    • Run

    ASK

    Answering free-text questions about the PR.

    [*]

    GENERATE CUSTOM LABELS

    Generates custom labels for the PR, based on specific guidelines defined by the user

    [*]

    TEST 💎

    Generates unit tests for a specific component, based on the PR code change.

    [*]

    CI FEEDBACK 💎

    Generates feedback and analysis for a failed CI job.

    [*]

    CUSTOM SUGGESTIONS 💎

    Generates custom suggestions for improving the PR code, based on specific guidelines defined by the user.

    [*]

    SIMILAR ISSUE

    Automatically retrieves and presents similar issues.

    [*]

    (1) Note that each tool be triggered automatically when a new PR is opened, or called manually by commenting on a PR.

    (2) Tools marked with [*] require additional parameters to be passed. For example, to invoke the /ask tool, you need to comment on a PR: /ask "<question content>". See the relevant documentation for each tool for more details.

    @delano delano self-assigned this Mar 24, 2024
    @delano delano marked this pull request as ready for review March 24, 2024 19:23
    delano added 2 commits March 24, 2024 12:30
    Signed-off-by: delano <delano@cpan.org>
    - Update Django REST Framework to latest main branch
    - Update drf-registration to latest commit
    - Update drfpasswordless to latest commit
    - Update various dependency versions
    Minor formatting changes
    
    All details:
    The Django REST Framework is updated from the local Git repository to the main branch of the remote repository (git@github.com:delano/django-rest-framework.git@main).
    The drf-registration package is updated from the local Git repository to a specific commit (git@github.com:delano/drf-registration.git@b6b50ef151473385b2fdc1b3800773545d689e3c).
    The drfpasswordless package is updated from the local Git repository to a specific branch (git@github.com:delano/django-rest-framework-passwordless.git@delano/20240209-email-context).
    Dependency version updates:
    
    asgiref is updated from 3.7.2 to 3.8.1.
    coverage is updated from 7.4.3 to 7.4.4.
    django-unfold is updated from 0.20.5 to 0.21.1.
    markdown is updated from 3.5.2 to 3.6.
    packaging is updated from 23.2 to 24.0.
    phonenumbers is updated from 8.13.31 to 8.13.32.
    pre-commit is updated from 3.6.2 to 3.7.0.
    pytest is updated from 8.0.2 to 8.1.1.
    referencing is updated from 0.33.0 to 0.34.0.
    traitlets is updated from 5.14.1 to 5.14.2.
    wheel is updated from 0.42.0 to 0.43.0.
    These changes are likely to update Django REST Framework and its related dependencies to their latest versions, as well as update other dependencies to their latest compatible versions.
    @delano delano merged commit ad57365 into main Mar 24, 2024
    0 of 2 checks passed
    @delano delano deleted the delano/2024-03-22-new-tool-config branch March 24, 2024 20:58
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    1 participant