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

About us #303

Merged
merged 8 commits into from
Sep 14, 2017
Merged

About us #303

merged 8 commits into from
Sep 14, 2017

Conversation

Cleop
Copy link
Member

@Cleop Cleop commented Jul 14, 2017

#299 Creates About Us page
#274 #271 (sub issues that make up #299)
If you want to view/test the page locally open the project in atom and then select the about file and open click open in browser.

@Cleop Cleop added the in-progress An issue or pull request that is being worked on by the assigned person label Jul 14, 2017
@Cleop Cleop self-assigned this Jul 14, 2017
@ghost ghost added this to the Sprint 1 milestone Aug 23, 2017
@Cleop Cleop changed the title WIP - About us About us Sep 7, 2017
@Cleop Cleop assigned iteles and SimonLab and unassigned Cleop Sep 7, 2017
@Cleop Cleop added awaiting-review An issue or pull request that needs to be reviewed and removed in-progress An issue or pull request that is being worked on by the assigned person labels Sep 7, 2017
@iteles iteles mentioned this pull request Sep 10, 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.

Think the link colour comment might be a blocker, other won't be 👍

Thanks @Cleop ! 🎉

</div>
<div>
<p class="pl3 b dib mv1 dib">1.</p>
<p class="dib pl2 i mv0 times-new-roman w-80 w-90-ns v-top pt1">
Copy link
Member

Choose a reason for hiding this comment

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

Not sure this looks right on a new line with this alignment but also not a blocker.
screen shot 2017-09-10 at 17 08 30

Copy link
Member Author

Choose a reason for hiding this comment

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

What view are you seeing it on a separate line? I did have an issue with this before but when I pushed up the view on mobile/ iPad and desktop was all on one line...

Copy link
Member

Choose a reason for hiding this comment

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

This was a full desktop view on my macbook in Chrome. I can open an issue on it 👍

<a href="#" class="dwyl-yellow">worldwide community of over 350 creative technologists.</a>
We believe in inclusivity and accessibility and want to enrich the
lives of everyone in our community. That’s why we made
<a href="https://www.focushub.co.uk/" class"dwyl-yellow">a home for our London-based team at Focus Hub.</a>
Copy link
Member

Choose a reason for hiding this comment

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

This dwyl-yellow is being overriden in my browser: Chrome (standard, not Canary) v. 60.0.3112.113
screen shot 2017-09-10 at 17 09 54

Probably because it's the only link in this section that points to an http link and is recognised by the browser as such. Reset possibly required for links universally at the top of the stylesheet (haven't checked to see if you have one!)

Copy link
Member

Choose a reason for hiding this comment

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

Have opened a separate issue on line height: #315

Copy link
Member Author

Choose a reason for hiding this comment

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

There was a missing "=" after class!

<h3 class="dwyl-yellow mt1">People 🤝</h3>
<p class="lh-copy">
Our people are our primary focus, not profit. We have a
<a href="#" class="dwyl-yellow">worldwide community of over 350 creative technologists.</a>
Copy link
Member

Choose a reason for hiding this comment

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

Not sure where this should link to. We have 400+ people now on github, but 'only' 134 have publicised their membership of the org on GH, so if you're not a member of dwyl already, you only see 134 when you navigate to: https://github.com/orgs/dwyl/people

Copy link
Member Author

@Cleop Cleop Sep 11, 2017

Choose a reason for hiding this comment

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

Not sure either... was following the designs. @markwilliamfirth any thoughts? Otherwise I'm happy to just remove this link but keep the text. Also happy to update the text to say 400 if that's the latest figure.

Copy link

Choose a reason for hiding this comment

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

Doesn't it link to the community page?

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'll make it go there, thanks 😊

</div>

</section>
<section class="dwyl-bg-mint center pt4-ns pb5-ns">
Copy link
Member

Choose a reason for hiding this comment

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

It would be great to get a nice big comment here (and in most sections) so we don't have to go section hunting, e.g.

<!-- ------------------------------
              CONTACT FORM
-------------------------------- -->

@iteles iteles removed their assignment Sep 10, 2017
@Cleop Cleop assigned Cleop and unassigned SimonLab Sep 13, 2017
@Cleop Cleop added in-progress An issue or pull request that is being worked on by the assigned person and removed awaiting-review An issue or pull request that needs to be reviewed labels Sep 13, 2017
@Cleop Cleop assigned SimonLab and unassigned Cleop Sep 13, 2017
@Cleop Cleop added awaiting-review An issue or pull request that needs to be reviewed and removed in-progress An issue or pull request that is being worked on by the assigned person labels Sep 13, 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.

LGTM, thanks for the changes @Cleop

Merging as I know @SimonLab is not available today and we should get this in for testing ahead of tomorrow.

@iteles iteles merged commit 7194072 into master Sep 14, 2017
@iteles iteles deleted the about-us branch September 14, 2017 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-review An issue or pull request that needs to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants