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.

Sign up for GitHub

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

Jump to bottom

Removed duplicate loading screen #237

Merged
merged 12 commits into from
Apr 3, 2019
Merged

Removed duplicate loading screen #237

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
82b3ac4
Removed duplicate loading screen
pcowgill Apr 1, 2019
7ea1b58
Remove unused Home component
pcowgill Apr 2, 2019
80642fb
Style empty my assets and wording fix
pcowgill Apr 2, 2019
bee0ad0
Capitalization and call assets land in UI
pcowgill Apr 2, 2019
815fcf9
Debugging native-base issue
pcowgill Apr 2, 2019
7c3962b
Updated LandForSaleList snapshot
pcowgill Apr 2, 2019
110233b
Removed use of the word asset in UI
pcowgill Apr 2, 2019
0718361
Removing console.log
marcelomorgado Apr 2, 2019
b93ec2a
Merge branch 'develop' into feature/no-loading-screen
marcelomorgado Apr 2, 2019
abf7838
Review request
marcelomorgado Apr 2, 2019
47fb80b
Updating comment
marcelomorgado Apr 2, 2019
c6015b7
Merge branch 'develop' into feature/no-loading-screen
pcowgill Apr 3, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions decentraland/App.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import { Action } from "tasit-sdk";
const { ConfigLoader } = Action;
import tasitSdkConfig from "./config/default";
import { checkBlockchain, showFatalError, recoverAccount } from "./helpers";
import { Root } from "native-base";
Copy link
Member Author

Choose a reason for hiding this comment

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

@marcelomorgado Hmm, are you sure removing this won't break the toasts in the account creation / funding flow? I can test that.

Copy link
Contributor

@marcelomorgado marcelomorgado Apr 2, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

You are right! I've just undone the removal.

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, thanks. I'll confirm those toasts are working in this branch anyway before merging after you added it back, since this is the branch that started messing with toasts even when Root was still in App.js

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm glad I checked. Those aren't working on any other screens now. So we'll have to add the Root change on every screen that uses a showInfo, etc. function if we want the toasts everywhere we used to have them.

I'd be potentially okay with removing them from the account creation and funding flow since there's the on-screen text now too, but we at least need the toasts for when the land purchase is complete to work. Maybe that happens back on the ListLandForSaleScreen though where we already have it now?

Copy link
Contributor

Choose a reason for hiding this comment

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

I've tested it before put Root again on the App component. I think that is desirable be able to create a toast from every component that we want, I don't have a strong opinion about keeping or removing account creation steps toasts.

I have to check if the toast from account creation flow will be shown on list land screen even without Root on the App component. Was that your question?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, we have few options now and that seems the better one. Any idea of the content that this screen will have?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd be potentially okay with removing them from the account creation and funding flow since there's the on-screen text now too, but we at least need the toasts for when the land purchase is complete to work. Maybe that happens back on the ListLandForSaleScreen though where we already have it now?

@marcelomorgado Oh actually, if we merge it as-is, the toasts for finishing buying a piece of land do show up back on the list land screen, so we would have all the toasts we absolutely need if we were to merge this with Root just on the ListLand screen and in App.js, which leads to no errors. Thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

Unfortunately, the toast will stop to work on all screen as before, but it's a good option too since we can merge this with no terrible side effects and work on the "coaching" screen and have the toast back to all screens soon.

Copy link
Member Author

Choose a reason for hiding this comment

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

I see you gave a thumbs up in response to the last message, so I will go forward with the merging as-is plan for now.

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately, the toast will stop to work on all screen as before, but it's a good option too since we can merge this with no terrible side effects and work on the "coaching" screen and have the toast back to all screens soon.

Okay, sounds good to me.

import { Ionicons } from "@expo/vector-icons";

const store = createStore(decentralandApp);
Expand Down Expand Up @@ -62,9 +61,7 @@ Is the 'config/default.js' file correct?`;
} else {
return (
<Provider store={store}>
<Root>
<AppNavigator />
</Root>
<AppNavigator />
</Provider>
);
}
Expand Down
8 changes: 3 additions & 5 deletions decentraland/AppNavigator.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import {
createAppContainer,
} from "react-navigation";

import HomeScreen from "./screens/HomeScreen";
import ListLandForSaleScreen from "./screens/ListLandForSaleScreen";
import BuyLandScreen from "./screens/BuyLandScreen";
import OnboardingHomeScreen from "./screens/OnboardingHomeScreen";
Expand Down Expand Up @@ -49,7 +48,6 @@ const headerWithBackButton = ({ navigation }) => {

const AssetsForSaleNavigator = createStackNavigator(
{
HomeScreen,
ListLandForSaleScreen: {
screen: ListLandForSaleScreen,
navigationOptions: headerWithMenuButton,
Expand All @@ -76,7 +74,7 @@ const AssetsForSaleNavigator = createStackNavigator(
},
},
{
initialRouteName: "HomeScreen",
initialRouteName: "ListLandForSaleScreen",
defaultNavigationOptions,
}
);
Expand All @@ -98,13 +96,13 @@ const MainDrawerNavigator = createDrawerNavigator({
AssetsForSale: {
screen: AssetsForSaleNavigator,
navigationOptions: {
drawerLabel: "Assets for sale",
drawerLabel: "Land for sale",
},
},
MyAssets: {
screen: MyAssetsNavigator,
navigationOptions: {
drawerLabel: "My Assets",
drawerLabel: "My land",
},
},
});
Expand Down
23 changes: 0 additions & 23 deletions decentraland/components/presentational/Home.js
View file
Open in desktop

This file was deleted.

10 changes: 0 additions & 10 deletions decentraland/components/presentational/Home.test.js
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion decentraland/components/presentational/LandForSaleList.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export default class LandForSaleList extends React.PureComponent {
const { length: listAmount } = landForSaleList;
const withoutAssetsForSale = !loadingInProgress && listAmount === 0;
return withoutAssetsForSale ? (
<LargeText>There are no assets for sale right now.</LargeText>
<LargeText>{`There isn't any land for sale right now.`}</LargeText>
) : (
<FlatList
data={landForSaleList}
Expand Down
11 changes: 9 additions & 2 deletions decentraland/components/presentational/MyAssetsList.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React from "react";
import { FlatList, StyleSheet } from "react-native";
import { FlatList, StyleSheet, View } from "react-native";
import PropTypes from "prop-types";
import Colors from "@constants/Colors";
import LargeText from "@presentational/LargeText";
Expand All @@ -15,7 +15,9 @@ export default class MyAssetsList extends React.PureComponent {
const { length: listAmount } = myAssetsList;
const withoutAssets = listAmount === 0;
return withoutAssets ? (
<LargeText>{`You haven't bought any land yet.`}</LargeText>
<View style={styles.emptyContainer}>
<LargeText>{`You haven't bought any land yet.`}</LargeText>
</View>
) : (
<FlatList
data={myAssetsList}
Expand All @@ -37,4 +39,9 @@ const styles = StyleSheet.create({
flex: 1,
backgroundColor: Colors.backgroundColor,
},
emptyContainer: {
flex: 1,
alignItems: "center",
justifyContent: "center",
},
});
27 changes: 0 additions & 27 deletions decentraland/components/presentational/__snapshots__/Home.test.js.snap
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion decentraland/components/presentational/__snapshots__/LandForSaleList.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

exports[`LandForSaleList renders the component after having loaded an empty list 1`] = `
<LargeText>
There are no assets for sale right now.
There isn't any land for sale right now.
</LargeText>
`;

Expand Down
16 changes: 13 additions & 3 deletions decentraland/components/presentational/__snapshots__/MyAssetsList.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,17 @@ exports[`MyAssetsList renders the component with assets 1`] = `
`;

exports[`MyAssetsList renders the component without assets 1`] = `
<LargeText>
You haven't bought any land yet.
</LargeText>
<Component
style={
Object {
"alignItems": "center",
"flex": 1,
"justifyContent": "center",
}
}
>
<LargeText>
You haven't bought any land yet.
</LargeText>
</Component>
`;
1 change: 1 addition & 0 deletions decentraland/helpers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ export const fundAccountWithMana = async accountAddress => {
// More about Toast component: https://docs.nativebase.io/Components.html#toast-def-headref
const showToast = msg =>
Toast.show({ text: msg, duration: 3000, buttonText: "Okay" });

export const showFatalError = msg => console.error(msg);
export const showError = msg => showToast(`ERROR: ${msg}`);
export const showWarn = msg => showToast(`WARN: ${msg}`);
Expand Down
2 changes: 1 addition & 1 deletion decentraland/screens/BuyLandScreen.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export class BuyLandScreen extends React.Component {
addToMyAssetsList(asset);
};

showInfo(`Buying the ${typeDescription.toLowerCase()} asset...`);
showInfo(`Buying the ${typeDescription.toLowerCase()}...`);
_executeOrder(landForSale, account, onSuccess);
navigation.navigate("ListLandForSaleScreen");
};
Expand Down
22 changes: 0 additions & 22 deletions decentraland/screens/HomeScreen.js
View file
Open in desktop

This file was deleted.

15 changes: 0 additions & 15 deletions decentraland/screens/HomeScreen.test.js
View file
Open in desktop

This file was deleted.

19 changes: 13 additions & 6 deletions decentraland/screens/ListLandForSaleScreen.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
showInfo,
getContracts,
} from "../helpers";
import { Root } from "native-base";

import DecentralandUtils from "tasit-sdk/dist/helpers/DecentralandUtils";

Expand All @@ -23,7 +24,7 @@ const { ESTATE, PARCEL } = AssetTypes;
export class ListLandForSaleScreen extends React.Component {
componentDidMount = async () => {
try {
showInfo("Loading assets for sale...");
showInfo("Loading land for sale...");
await this._loadAssetsForSale();
} catch (err) {
showError(err);
Expand All @@ -40,6 +41,7 @@ export class ListLandForSaleScreen extends React.Component {
const { getOpenSellOrders } = decentralandUtils;

const fromBlock = 0;

const openSellOrdersEvents = await getOpenSellOrders(fromBlock);

let contracts = getContracts();
Expand Down Expand Up @@ -169,12 +171,17 @@ export class ListLandForSaleScreen extends React.Component {
const { assetsForSale } = this.props;
const { list, loadingInProgress } = assetsForSale;

// Note: The initial route component from react-native-navigation
// Should add the NativaBase `Root` component.
// See more: https://github.com/tasitlabs/tasit/pull/237#issuecomment-479124236
return (
<LandForSaleList
landForSaleList={list}
renderItem={this._renderItem}
loadingInProgress={loadingInProgress}
/>
<Root>
<LandForSaleList
landForSaleList={list}
renderItem={this._renderItem}
loadingInProgress={loadingInProgress}
/>
</Root>
);
}
}
Expand Down
10 changes: 0 additions & 10 deletions decentraland/screens/__snapshots__/HomeScreen.test.js.snap
View file
Open in desktop

This file was deleted.

12 changes: 7 additions & 5 deletions decentraland/screens/__snapshots__/ListLandForSaleScreen.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`ListLandForSaleScreen renders the component 1`] = `
<LandForSaleList
landForSaleList={Array []}
loadingInProgress={true}
renderItem={[Function]}
/>
<Styled(Root)>
<LandForSaleList
landForSaleList={Array []}
loadingInProgress={true}
renderItem={[Function]}
/>
</Styled(Root)>
`;