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

Created ProtectYourself, LocalMaps, and Distress pages with menu bar #114

Closed

Conversation

@TasfiaAddrita
Copy link
Collaborator

TasfiaAddrita commented Mar 21, 2020

Hi! As the title suggests, I created pages for the "Protect Yourself" and "Local Maps" for the buttons in the "Menu" page and added the back-to-menu button on each page. Navigation was a bit annoying, especially if you clicked one of the buttons by mistake.

I also added "ios/" to the .gitignore file since that will generate if you run the app on Xcode.

I hope I did this okay, I'm a college student and new to open source. I would appreciate any feedback, I'm sure I made a mistake or can improve somewhere. This is a great learning experience and I want to make the best of it.

One question - Should I be testing my changes for Android as well?

@TasfiaAddrita TasfiaAddrita requested a review from hspinks Mar 21, 2020
@TasfiaAddrita TasfiaAddrita changed the title Created Protect_Yourself and Local_Maps pages with menu bar Created ProtectYourself and LocalMaps pages with menu bar Mar 21, 2020
.gitignore Outdated Show resolved Hide resolved
client/ionic/src/pages/LocalMaps.tsx Outdated Show resolved Hide resolved
@advayDev1

This comment has been minimized.

Copy link
Collaborator

advayDev1 commented Mar 21, 2020

Thanks for the contribution! Posted a few comments.

@TasfiaAddrita TasfiaAddrita changed the title Created ProtectYourself and LocalMaps pages with menu bar Created ProtectYourself, LocalMaps, and Distress pages with menu bar Mar 22, 2020
@TasfiaAddrita TasfiaAddrita requested a review from advayDev1 Mar 22, 2020
client/ionic/src/App.tsx Outdated Show resolved Hide resolved
client/ionic/src/App.tsx Outdated Show resolved Hide resolved
Copy link
Collaborator

myiremark left a comment

Thanks for helping! please dont forget to add your name and email to the contributors file

const LocalMaps: React.FC = () => {
return (
<IonPage>
<IonHeader>

This comment has been minimized.

Copy link
@myiremark

myiremark Mar 22, 2020

Collaborator

There's a TopNav component in

https://github.com/WorldHealthOrganization/app/blob/master/client/ionic/src/components/TopNav.tsx

An Example implementation is at:

Please refactor to include and reflect DRY.

@@ -0,0 +1,24 @@
import React from 'react';

This comment has been minimized.

Copy link
@myiremark

myiremark Mar 22, 2020

Collaborator

please rebase to reflect the latest version

@@ -0,0 +1,24 @@
import React from 'react';

This comment has been minimized.

Copy link
@myiremark

myiremark Mar 22, 2020

Collaborator

Please rebase to reflect the latest code

@TasfiaAddrita

This comment has been minimized.

Copy link
Collaborator Author

TasfiaAddrita commented Mar 23, 2020

It looks like my code is outdated and a navbar component has already been created so I'm just going to close this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants
You can’t perform that action at this time.