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

Remove all those warnings #615

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Remove all those warnings #615

wants to merge 2 commits into from

Conversation

utunga
Copy link
Contributor

@utunga utunga commented May 22, 2020

Small fixes in a few places - mostly removing things that were generating warnings.

They're all minor but if we don't remove them then the warnings become useless..

Before:
image

After:
image

--

Also I had to change <A> to <Button> to avoid an accessibility warning. And fixed the spelling/changed 'prefered' to 'suggested' while I was at it.

Before

image

*After

image

Not happy with the spacing of the 'no thanks!' but in the end it was taking too long to fix and its not worse than what was there before!

…g an executive decision on this one instead of opening a

long slack thread - 'suggested' much more generic than 'prefered'.
decisions but if it looks like people may want to reuse this code soon
used comments instead of outright delete
</Row>
);
};
// const MissionStatusRow = ({ classes, mission }) => {
Copy link
Member

Choose a reason for hiding this comment

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

is this used anymore? Should it just be removed?

@@ -13,7 +13,6 @@ import DashboardIcon from "@material-ui/icons/Dashboard";
import DoubleArrowIcon from "@material-ui/icons/DoubleArrow";
import ExitToApp from "@material-ui/icons/ExitToApp";
import MenuIcon from "@material-ui/icons/Menu";
import MoveToInboxIcon from "@material-ui/icons/MoveToInbox";
Copy link
Member

Choose a reason for hiding this comment

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

Latest master uses this icon so need to add back

@@ -21,7 +21,7 @@ import { User, useOrganization } from "../../model";
import Mission from "../../model/Mission";
import { routes } from "../../routing";
import { useFirestoreConnect } from "react-redux-firebase";
import FoodBoxDeliveryIcon from "../../component/icons/FoodBoxDeliveryIcon";
//import FoodBoxDeliveryIcon from "../../component/icons/FoodBoxDeliveryIcon";
Copy link
Member

Choose a reason for hiding this comment

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

just delete?

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.

2 participants