Skip to content

[HOLD for payment 2024-11-14] Migrate MapView to useOnyx #50343

Closed

Description

@neil-marcellini

Problem

Coming from this PR ES Lint changed files check is failing because we need to migrate to useOnyx

#41242 (comment)

Solution

Do the migration

Issue OwnerCurrent Issue Owner: @twisterdotcom

Activity

  1. added
    BugSomething is broken. Auto assigns a BugZero manager.
    on Oct 7, 2024
  2. melvin-bot commented on Oct 7, 2024

    @melvin-bot

    Triggered auto assignment to @twisterdotcom (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

  3. neil-marcellini commented on Oct 7, 2024

    @neil-marcellini
    ContributorAuthor

    @janicduplessis is going to take this one since he touched the code that prompted the error.

  4. janicduplessis commented on Oct 7, 2024

    @janicduplessis
    Contributor

    I will take this!

  5. Shahidullah-Muffakir commented on Oct 7, 2024

    @Shahidullah-Muffakir
    Contributor

    Proposal

    Please re-state the problem that we are trying to solve in this issue.

    We are trying to migrate the MapView component from using the withOnyx HOC to the newer useOnyx hook.

    What is the root cause of that problem?

    The current implementation uses withOnyx, which is an older pattern for connecting components to Onyx data.

    What changes do you think we should make in order to solve the problem?

    In this file:

    const MapView = forwardRef<MapViewHandle, ComponentProps>(

    1. Remove the withOnyx HOC from the MapView component.
    2. Add the useOnyx hook inside the MapView component to fetch the userLocation data.

    Add this code inside the component to get userLocation:

    const [userLocation] = useOnyx({
        key: ONYXKEYS.USER_LOCATION,
    });
    
    
  6. cretadn22 commented on Oct 7, 2024

    @cretadn22
    Contributor

    Proposal

    Please re-state the problem that we are trying to solve in this issue.

    Migrate MapView to useOnyx

    What is the root cause of that problem?

    NA

    What changes do you think we should make in order to solve the problem?

    export default withOnyx<ComponentProps, MapViewOnyxProps>({
    userLocation: {
    key: ONYXKEYS.USER_LOCATION,
    },

    We need to remove the withOnyx and use const [userLocation] = useOnyx(ONYXKEYS. USER_LOCATION);

    What alternative solutions did you explore? (Optional)

  7. janicduplessis commented on Oct 8, 2024

    @janicduplessis
    Contributor

    @Shahidullah-Muffakir proposal looks good if you want to take this over.

  8. Shahidullah-Muffakir commented on Oct 8, 2024

    @Shahidullah-Muffakir
    Contributor

    @Shahidullah-Muffakir proposal looks good if you want to take this over.

    @janicduplessis ,Sure, I can submit a PR. Thank you.

  9. added
    ReviewingHas a PR in review
    and removed on Oct 8, 2024
  10. 5 remaining items

  11. neil-marcellini commented on Oct 30, 2024

    @neil-marcellini
    ContributorAuthor

    Assigning @Ollyws as C+

  12. added
    Awaiting PaymentAuto-added when associated PR is deployed to production
    and removed on Nov 7, 2024
  13. changed the title [-]Migrate MapView to useOnyx[/-] [+][HOLD for payment 2024-11-14] Migrate MapView to useOnyx[/+] on Nov 7, 2024
  14. melvin-bot commented on Nov 7, 2024

    @melvin-bot

    Reviewing label has been removed, please complete the "BugZero Checklist".

  15. melvin-bot commented on Nov 7, 2024

    @melvin-bot

    The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.58-2 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

    If no regressions arise, payment will be issued on 2024-11-14. 🎊

    For reference, here are some details about the assignees on this issue:

  16. melvin-bot commented on Nov 7, 2024

    @melvin-bot

    @Ollyws @twisterdotcom The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

  17. melvin-bot commented on Nov 14, 2024

    @melvin-bot

    Payment Summary

    Upwork Job

    BugZero Checklist (@twisterdotcom)

    • I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
    • I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants//hired)
    • I have paid out the Upwork contracts or cancelled the ones that are incorrect
    • I have verified the payment summary above is correct
  18. Ollyws commented on Nov 14, 2024

    @Ollyws
    Contributor

    Requested in ND.

  19. garrettmknight commented on Nov 26, 2024

    @garrettmknight
    Contributor

    $250 approved for @Ollyws

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

Awaiting PaymentAuto-added when associated PR is deployed to productionBugSomething is broken. Auto assigns a BugZero manager.DailyKSv2

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions