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

Perceived performance improvement with Mapbox Static Images API #331

Open
wants to merge 28 commits into
base: release/v1.1.0
Choose a base branch
from

Commits on Oct 5, 2022

  1. Mapbox Component (#267)

    This PR introduces the Mapbox Component. This lays the basic foundation for the component, with the interfaces constructed through discussion with Product.
    
    Here are two relevant slack threads:
    https://yext.slack.com/archives/C032CKFARGS/p1658322867803349 (inception)
    https://yext.slack.com/archives/C032CKFARGS/p1659449732357219 (more recent)
    
    J=SLAP-2221
    TEST=manual
    
    Tested through LocationsPage
    
    Later items will add jest tests and storybook for further testing
    
    NOTE: as of now, local test-site requires an `env` variable REACT_APP_MAPBOX_API_KEY for map to work properly
    For github workflow to work (in future test items), some changes may be needed in the workflow: facebook/create-react-app#9064 (comment)
    alextaing authored Oct 5, 2022
    Configuration menu
    Copy the full SHA
    ab88584 View commit details
    Browse the repository at this point in the history

Commits on Oct 10, 2022

  1. mapbox component jest tests (#311)

    added jest tests for Mapbox component, specifically for the two prop fields: `getCoordinate` and `onDrag`. The remaining props are best tested through storybook.
    
    note that `mapbox-gl` is mocked as it seems to use some web browser functionality on initialization that is not supported in jest test environment (jsdom). Without mocking, jest would present an error `Error: Failed to initialize WebGL.`.
    - tried third party libraries `jest-webgl-canvas-mock` and `jest-canvas-mock` still result in more window properties access related errors (`[TypeError: e.window.Worker is not a constructor]`, which could be resolve by [manually mocking Worker](jestjs/jest#3449), `[TypeError: this.target.addEventListener is not a function]`...).
    - Looked into how pageJS test mapbox provider in their map component: the map wrapper component invoke a [load function](https://github.com/yext/pages/blob/main/packages/pages/src/components/map/map.tsx#L112) to construct [a script tag](https://github.com/yext/components-tsx-maps/blob/main/src/Providers/Mapbox.js#L137) from `yext/components-tsx-maps` and the assertions in the unit tests seem to check for the rendering of the wrapper component only and not waiting for the script to load. No content/interaction of the map was tested from my understanding.
    - Looked into how mapbox-gl-js does their own testing: they had a couple util files to set up the environment for their tests (mock requests, [mock HtmlCanvas/WebLG in window](https://github.com/mapbox/mapbox-gl-js/blob/main/src/util/window.js) object, etc.).
    
    I decided this complexity is probably unnecessary to add to the repo but we can discuss if other may think differently. We can have jest strictly test new isolated functionalities added from the component and leave all the UI rendering and mapbox interaction to storybook tests.
    
    SLAP-2222
    TEST=auto
    
    new jest tests passed
    yen-tt authored Oct 10, 2022
    Configuration menu
    Copy the full SHA
    8c677c6 View commit details
    Browse the repository at this point in the history

Commits on Oct 11, 2022

  1. add react-dom as peer dep (#313)

    Add react-dom as a peer dependency as it's now use in Mapbox component.
    
    Note that, ReactDOM.render is deprecated in React 18, which now have a new client render function `createRoot`. For now, a warning will display in console informing user that the app will behave as if it’s running React 17 until the code switch to the new API. A new item will be created to address this issue.
    
    TEST=manual
    
    `npm pack` the component lib. Created a new React app using react 17/18, see that react-dom is installed as peer dep (automatically install with npm >=7) and mapbox component is display as expected (there's a warning in React 18).
    yen-tt authored Oct 11, 2022
    Configuration menu
    Copy the full SHA
    69327c1 View commit details
    Browse the repository at this point in the history

Commits on Oct 18, 2022

  1. docs(MapboxMap): add storybook stories (#312)

    single marker story
    multiple marker story
    custom marker story - follow docs.mapbox.com guide to add a simple popup pin
    
    J=SLAP-2223
    TEST=manual
     
    serve and view stories
    tatimblin authored Oct 18, 2022
    Configuration menu
    Copy the full SHA
    731c9bd View commit details
    Browse the repository at this point in the history

Commits on Oct 28, 2022

  1. use createRoot when possible for rendering pin components (#319)

    This PR uses a dynamic imports with a .catch fallback for when react-dom/client does not exist.
    
    MapboxMap code had to be tweaked due to differences between ReactDOM.render and ReactDOM.createRoot().render(). Namely, createRoot() does not modify the container element unless it already exists on the physical page. This means that we can't call document.creatElement('div'), use createRoot, and THEN attach it to the mapbox map. Instead, we have to attach the div to the map first.
    
    This also seems it make it more difficult to unit test, my guess would be because createRoot does not do anything on the server. For now I use some jest.mocks to ensure the right methods are being called in the right environments.
    
    TEST=manual,auto
    
    storybook can start up (react 17)
    test site can display custom pin in both react 17 and 18
    oshi97 authored Oct 28, 2022
    Configuration menu
    Copy the full SHA
    0d78e7e View commit details
    Browse the repository at this point in the history

Commits on Nov 4, 2022

  1. Configuration menu
    Copy the full SHA
    f585a30 View commit details
    Browse the repository at this point in the history

Commits on Nov 9, 2022

  1. Configuration menu
    Copy the full SHA
    2c8a500 View commit details
    Browse the repository at this point in the history

Commits on Nov 10, 2022

  1. function for url, change interface

    Yen Truong committed Nov 10, 2022
    Configuration menu
    Copy the full SHA
    a926516 View commit details
    Browse the repository at this point in the history
  2. add story for static image before load

    Yen Truong committed Nov 10, 2022
    Configuration menu
    Copy the full SHA
    7b89a4f View commit details
    Browse the repository at this point in the history
  3. refactor from hook to component

    Yen Truong committed Nov 10, 2022
    Configuration menu
    Copy the full SHA
    0bb3220 View commit details
    Browse the repository at this point in the history
  4. mock response to be error instead of delay

    Yen Truong committed Nov 10, 2022
    Configuration menu
    Copy the full SHA
    261facb View commit details
    Browse the repository at this point in the history
  5. Configuration menu
    Copy the full SHA
    8faf428 View commit details
    Browse the repository at this point in the history
  6. wait for map to load for non static map stories

    Yen Truong committed Nov 10, 2022
    Configuration menu
    Copy the full SHA
    28727e4 View commit details
    Browse the repository at this point in the history
  7. ts

    Yen Truong committed Nov 10, 2022
    Configuration menu
    Copy the full SHA
    bb25f48 View commit details
    Browse the repository at this point in the history
  8. update percy config in story

    Yen Truong committed Nov 10, 2022
    Configuration menu
    Copy the full SHA
    91089ce View commit details
    Browse the repository at this point in the history
  9. test

    Yen Truong committed Nov 10, 2022
    Configuration menu
    Copy the full SHA
    3d5a491 View commit details
    Browse the repository at this point in the history

Commits on Nov 14, 2022

  1. hide getCoordinate and onDrag control

    Yen Truong committed Nov 14, 2022
    Configuration menu
    Copy the full SHA
    2c70739 View commit details
    Browse the repository at this point in the history
  2. for snapshot of dynamic map with play function

    Yen Truong committed Nov 14, 2022
    Configuration menu
    Copy the full SHA
    8ad916a View commit details
    Browse the repository at this point in the history

Commits on Nov 15, 2022

  1. Merge branch 'develop' into dev/mapbox-static-image

    Yen Truong committed Nov 15, 2022
    Configuration menu
    Copy the full SHA
    7ab0c0c View commit details
    Browse the repository at this point in the history
  2. update package lock for test-site

    Yen Truong committed Nov 15, 2022
    Configuration menu
    Copy the full SHA
    44f0e4a View commit details
    Browse the repository at this point in the history
  3. remove play function -- doesnt deplay snapshot

    Yen Truong committed Nov 15, 2022
    Configuration menu
    Copy the full SHA
    def501e View commit details
    Browse the repository at this point in the history

Commits on Nov 16, 2022

  1. test dev wcag gh workflow with mapbox api key input

    Yen Truong committed Nov 16, 2022
    Configuration menu
    Copy the full SHA
    3538bfb View commit details
    Browse the repository at this point in the history
  2. test

    Yen Truong committed Nov 16, 2022
    Configuration menu
    Copy the full SHA
    57f505f View commit details
    Browse the repository at this point in the history
  3. remove logs

    Yen Truong committed Nov 16, 2022
    Configuration menu
    Copy the full SHA
    139ee22 View commit details
    Browse the repository at this point in the history
  4. update workflow to point back to v1 branch, run WCAG in feature branc…

    …h as well
    Yen Truong committed Nov 16, 2022
    Configuration menu
    Copy the full SHA
    6b48f80 View commit details
    Browse the repository at this point in the history

Commits on Nov 18, 2022

  1. Fix wcag github action mapbox issues (#337)

    paired with the changes in this [PR](yext/slapshot-reusable-workflows#22) in WCAG workflow, this PR updates the WCAG github action in the repo to pass in the mapbox key. Also updated the wcag test to exclude checking elements coming from mapbox canvas container as any potential violations coming from there is outside of our repo's control.
    
    WCAG github action also run on pull request to feature branch now.
    
    J=SLAP-2458
    TEST=auto
    
    see that WCAG github action now passes
    yen-tt authored and Yen Truong committed Nov 18, 2022
    Configuration menu
    Copy the full SHA
    06c7725 View commit details
    Browse the repository at this point in the history
  2. Fix visual coverage test in Github Actions (#339)

    In addition to the changes in the [workflow PR](yext/slapshot-reusable-workflows#24), this up updates run-tests github action and coverage github action to pass mapbox api key so visual coverage test works as expected.
    
    J=SLAP-2467
    TEST=auto
    
    see that run-tests github action and coverage github action passed -- looked into the logs, no false positive / errors related to visual coverage.
    yen-tt authored and Yen Truong committed Nov 18, 2022
    Configuration menu
    Copy the full SHA
    1ead573 View commit details
    Browse the repository at this point in the history
  3. periods

    Yen Truong committed Nov 18, 2022
    Configuration menu
    Copy the full SHA
    ffe64e5 View commit details
    Browse the repository at this point in the history