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

added question pages #10

Merged
merged 4 commits into from
Jul 24, 2017
Merged

added question pages #10

merged 4 commits into from
Jul 24, 2017

Conversation

armydar
Copy link
Collaborator

@armydar armydar commented Jul 17, 2017

No description provided.

@abiodun0
Copy link
Collaborator

@armydar Can you update this PR with screen shots?

@abiodun0 abiodun0 closed this Jul 18, 2017
@abiodun0 abiodun0 reopened this Jul 18, 2017
@armydar
Copy link
Collaborator Author

armydar commented Jul 18, 2017 via email

@armydar
Copy link
Collaborator Author

armydar commented Jul 18, 2017

BEGIN

begin

QUESTION 1

question 1

QUESTION 2

question 2

QUESTION 3

question 3

@armydar
Copy link
Collaborator Author

armydar commented Jul 18, 2017

HOW TO JOIN

screengrab

Copy link
Collaborator

@abiodun0 abiodun0 left a comment

Choose a reason for hiding this comment

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

The logo as it appears seems too big. This should be made to be a regular navigation local like in popular websites. You can use devcenter.co as a guide when doing this.

Overall Good job. But this seem to be designed without atheistic and making it visually appealing in mind. Please put this into consideration when implementing the feedbacks. add a css file or use a css framework will be a +

@@ -0,0 +1,18 @@
<!DOCTYPE html>
<html>
<head>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Code Quality: Critical
This seems to be designed without any use of css or care for responsiveness. This should be refactored and appropriate css should be used.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes...no styling as the #4 is basically to create plain html

</header>
</head>
<body>
<section>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Code Quality: Nitpick
This should be revisited and divided into reasonable divs

@osioke
Copy link
Contributor

osioke commented Jul 19, 2017

@abiodun0 this PR addresses just the HTML pages, in the next task she is to add the CSS files, and beautify it.

@armydah to help make this clearer on your next pull request (PR), you could do the following:

  • The name of PR should show what the PR is about. In this case the PR is a solution/fix for a named issue, so you should name your PR to show that it is a fix for that issue. For example you could say Added plain HTML files for the pages to fix #4 or Plain HTML pages created to fix #4. Writing the no of the issue after a pound sign (#) helps mention that issue.

Also @armydar what @abiodun0 is saying is the way you wrote the HTML and how you structured the divs and other tags can be done better, and the images you added are not resized and thus they appear too big, and make the site look unappealing. You should fix the image, and check the tags you used and how you can reduce or use them better.

I also checked the files, and noticed a few things.

  • The HTML file for the first page (the landing page) is not included here.
  • The files should be arranged together in a folder, this will help create a folder structure, which helps organize and keep your files easily understandable by a browser.
  • The image folder is not included in this pull request, making the image not load if accessed outside your laptop
  • The buttons do not link to the other pages, that is not links from one page to the other. And so one cannot go through the pages as if using the app. You can read the flow of the app starting here to see how the app works, and how each page is supposed to link to each other.
  • The questions are not meant to be entered on the page using HTML, they are to be pulled from the database using JavaScript, and you would address this in a later task. And so the question pages are meant to be template pages. See this explainer card to understand better. You can ask @abiodun0 how you can create the pages, or if you are supposed to create them now.

Good one generally, I will drop my finally feedback on the Slack channel.

@osioke osioke closed this Jul 19, 2017
@osioke osioke reopened this Jul 19, 2017
@abiodun0
Copy link
Collaborator

@osioke Awesome .. those this means i can merge this request?

@osioke
Copy link
Contributor

osioke commented Jul 19, 2017

At all @abiodun0, she still has to do the landing page, group all the front-end work in a folder on the repo, link the pages, add the image folder, and redo the template page for the questions and resource. cc: @armydah

@armydar
Copy link
Collaborator Author

armydar commented Jul 23, 2017

fixed issue #5 @abiodun0

@abiodun0 abiodun0 merged commit e6fb8d1 into devcenter-square:master Jul 24, 2017
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