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

Add a collection of beauty fixes #121

Merged
merged 3 commits into from
Sep 4, 2019
Merged

Add a collection of beauty fixes #121

merged 3 commits into from
Sep 4, 2019

Conversation

karolinkas
Copy link
Contributor

@karolinkas karolinkas commented Aug 23, 2019

Description

Add a collection of style/copy improvements.

Changes 🏗

  • Improve EtherBanner paddings / spacing / casing
  • Increase size of code image
  • Use GridItem for TLDR section

Also closes #127

@ceolson01 ceolson01 force-pushed the beautyfixes branch 2 times, most recently from e189b3c to 63f8d03 Compare August 27, 2019 23:06
@ceolson01 ceolson01 self-assigned this Aug 27, 2019
@ceolson01 ceolson01 requested a review from ryanchristo August 28, 2019 00:53
@ceolson01 ceolson01 added bug Something isn't working enhancement New feature or request needs-code-review and removed bug Something isn't working labels Aug 28, 2019
@ceolson01 ceolson01 added this to the Sprint 33 milestone Aug 28, 2019
Copy link
Contributor

@ryanchristo ryanchristo left a comment

Choose a reason for hiding this comment

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

Nice work! I like that we are reusing the GridItem for the TLDR now. 💪

I'm not entirely sure that we should increase the height of the code snippet image but rather we should make the code easier to read (take a new snapshot with increased font size for the code in the editor).

@@ -8,6 +8,6 @@

.image {
margin-bottom: 0;
max-height: 90%;
max-height: 98%;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think @collinvine's request for this to be larger was more along the lines of the code itself being larger rather than increasing the height. I think we need to take another screenshot. 😬 I told @reneegranillo that it would be ok to cut off the code wherever we need to as long as createColony was included.

@collinvine can you clarify your request?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe one of these might be better?

Screenshot from 2019-08-28 12-48-43

Screenshot from 2019-08-28 12-50-00

Choose a reason for hiding this comment

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

That's right, @ryanchristo. People viewing the website should easily be able to read the code.

The included examples are better, but I'd suggest that even they are too small.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok. If it gets any bigger, we will cut off the createColony step so we will need to be more specific about what we want to show and whether we want it to show usable code.

@ceolson01 ceolson01 force-pushed the beautyfixes branch 2 times, most recently from 42bbc7d to 9f7e19d Compare August 28, 2019 19:21
@ceolson01
Copy link
Contributor

@ryanchristo The image has been updated to use the larger of the two that you provided.

cc @reneegranillo and @collinvine for approval as well

Screen Shot 2019-08-28 at 2 30 32 PM

Copy link
Contributor

@ryanchristo ryanchristo left a comment

Choose a reason for hiding this comment

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

This looks good to me although @reneegranillo and @collinvine should approve as well. I imagine @reneegranillo might want the screenshot to have the same framing as the dApp screenshot.

Also, I would be happy to update the code snippet that we are showing if anyone wants to weigh in on what we should emphasize. I decided to use the code from colonyJS "Get Started" which seemed like the simplest choice at the time but we can change it if we want to show something specific.

@collinvine
Copy link

Good by me.

@ceolson01 ceolson01 merged commit 46c49aa into generator Sep 4, 2019
@ceolson01 ceolson01 deleted the beautyfixes branch September 4, 2019 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Website - Homepage TLDR content spacing
4 participants