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

Creates Community page #290

Merged
merged 4 commits into from
Jun 24, 2017
Merged

Creates Community page #290

merged 4 commits into from
Jun 24, 2017

Conversation

Cleop
Copy link
Member

@Cleop Cleop commented Jun 23, 2017

Creates community page #289:

  • Top image banner
  • White copy section
  • Responsive

@Cleop Cleop added the awaiting-review An issue or pull request that needs to be reviewed label Jun 23, 2017
@dwylbot
Copy link

dwylbot bot commented Jun 23, 2017

@Cleop, a reviewer has been added to the pull request.
The pull request looks ready for review (no "in-progress" label).
So the reviewer has been added as an assignee and the "awaiting-reivew" label as been added to.

Any questions, complaints, feedback, contributions?
Discuss
If you prefer, you can also send us anonymous feedback: https://dwyl-feedback.herokuapp.com/feedback/new

1 similar comment
@dwylbot
Copy link

dwylbot bot commented Jun 23, 2017

@Cleop, a reviewer has been added to the pull request.
The pull request looks ready for review (no "in-progress" label).
So the reviewer has been added as an assignee and the "awaiting-reivew" label as been added to.

Any questions, complaints, feedback, contributions?
Discuss
If you prefer, you can also send us anonymous feedback: https://dwyl-feedback.herokuapp.com/feedback/new

@dwylbot dwylbot bot added awaiting-review An issue or pull request that needs to be reviewed labels Jun 23, 2017
@dwylbot
Copy link

dwylbot bot commented Jun 23, 2017

@Cleop, a reviewer has been added to the pull request.
The pull request looks ready for review (no "in-progress" label).
So the reviewer has been added as an assignee and the "awaiting-reivew" label as been added to.

Any questions, complaints, feedback, contributions?
Discuss
If you prefer, you can also send us anonymous feedback: https://dwyl-feedback.herokuapp.com/feedback/new

@ghost
Copy link

ghost commented Jun 23, 2017

lol @dwylbot[bot]

@@ -0,0 +1,138 @@
<!DOCTYPE html>
<html>
Copy link
Member

Choose a reason for hiding this comment

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

I hadn't noticed before but we're missing the lang attribute - useful for screen readers.
https://www.w3.org/International/questions/qa-html-language-declarations#attributes

<div class="dib-ns bg-white pt4 pa2 shadow-4 ml5-ns w-40-ns">

<form id="gform" method="POST" class="tc tl-ns" action="https://script.google.com/macros/s/AKfycby5ZdD8HfCaz1tMVLw26qYQKEXkP8qdfwLek3TH_RYH3l1o1-Li/exec">
<!-- change the form action to your script url -->
Copy link
Member

Choose a reason for hiding this comment

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

I think we don't need this comment tag any more 😊

</nav>

<section class="pt5 pt4-ns">
<img src="./img/common/community-cover.png" alt="dwyl community members github profile collage" class="w-100"/>
Copy link
Member

Choose a reason for hiding this comment

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

This image is pretty massive at 1.42MB, can we serve up a smaller version for m and mobile screens? These are the most likely to be on data or poor connections.

Either way, we should bring down the size of this image across the board, I suggest jpeg mini lite 👍

Copy link
Member

Choose a reason for hiding this comment

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

where did the image come from...? 🤔

Copy link
Member

Choose a reason for hiding this comment

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

@nelsonic I assume from the work you were doing over the weekend!

Copy link
Member

Choose a reason for hiding this comment

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

right ... but how did it end up here...? 🙃

Copy link
Member

Choose a reason for hiding this comment

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

Found it: #282 (comment) 😕
I already have a specific purpose for the output of my work and it's not this.
but i'm not going to "make a fuss" about it; that's what I get for making my output public.

We have people contributing remotely to our open-source projects from
Angola, Argentina, Australia, Bali, Belgium, Brazil, Canada, China,
Colombia, Estonia, Kenya, New Zealand, The Netherlands, Portugal, South
Africa and the USA.
Copy link
Member

Choose a reason for hiding this comment

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

and more! Better to say 'countries like' instead of just these countries?

<em>Thanks</em> for contacting us! We will get back to you soon!</h2>
</div>

<!-- Submit the Form to Google Using "AJAX" -->
Copy link
Member

Choose a reason for hiding this comment

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

Same again.

@iteles iteles added in-review Issue or pull request that is currently being reviewed by the assigned person and removed awaiting-review An issue or pull request that needs to be reviewed in-review Issue or pull request that is currently being reviewed by the assigned person labels Jun 23, 2017
Copy link
Member

@iteles iteles left a comment

Choose a reason for hiding this comment

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

No real blockers for me here, I'll add an issue for something I just noticed on the thank you message, but other than that LGTM.

@iteles iteles removed their assignment Jun 23, 2017
Copy link
Member

@nelsonic nelsonic left a comment

Choose a reason for hiding this comment

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

@Cleop looks great. 👍
agree with @iteles' notes.
(tho can be addressed in future issues/PRs...)

@nelsonic nelsonic removed their assignment Jun 23, 2017
@iteles iteles merged commit 6a10ed6 into master Jun 24, 2017
@iteles iteles deleted the community-page branch June 24, 2017 08:20
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.

4 participants