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

refactor: marketplace #528

Merged
merged 2 commits into from
Oct 8, 2018
Merged

Conversation

cazala
Copy link
Member

@cazala cazala commented Oct 8, 2018

This PR refactors the tab into a query param asset_type.

This keeps it consistent with the server API, and it lets us use the logic already implemented in the Location class to build paths and extract values from the router.

Also it allow us to have a single route for the marketplace (no need for marketplaceDefaultPage location)

This PR also refactores the MarketplacePage to work with assets instead of parcels and estates, which reduces repeated logic.

This PR also fixes a race condition that happened when the MarketplacePage was mounted and the publications for each asset type were fetched: now it relies on the router instead of the order that the actions are dispatched.

Copy link
Contributor

@nachomazzara nachomazzara left a comment

Choose a reason for hiding this comment

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

I like the way of getting the type with location 🥇

debounce={index * 100}
/>
))}
{assets.map(
Copy link
Contributor

@nachomazzara nachomazzara Oct 8, 2018

Choose a reason for hiding this comment

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

We could use AssetCard here

@@ -23,15 +22,15 @@ export function marketplaceReducer(state = INITIAL_STATE, action) {
[assetType]: total
}
}
let newGrid = action.isGrid
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it ok if we change it to const ?

@eordano
Copy link
Member

eordano commented Oct 8, 2018

looks good!

@nachomazzara nachomazzara merged commit 02eba0f into feat/simplify-fetch-pubs Oct 8, 2018
@nachomazzara nachomazzara deleted the refactor/marketplace branch October 8, 2018 13:14
cazala pushed a commit that referenced this pull request Oct 8, 2018
* feat: simplify actions, use /marketplace for pubs and inactive tab at marketplace

* refactor: marketplace (#528)

* refactor: marketplace

* chore: applied changes after review

* fix: render badge ar markaetplace
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants