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

Lists [fixes #767] #839

Merged
merged 4 commits into from
Mar 19, 2020
Merged

Lists [fixes #767] #839

merged 4 commits into from
Mar 19, 2020

Conversation

carlfairclough
Copy link
Contributor

@carlfairclough carlfairclough commented Mar 18, 2020

Added lists & documentation for them Implemented simple list on /developers page

Description

Added multiple list variations based on the Figma Style Guide. These include lists with descriptions, and list cards in multiple layouts.

An array of objects should be passed to leverage the card format, and a class should be added to the standard lists to enable their modification.

Screenshots (if appropriate):

Style Guide Documentation — Lists   Ethereum org
Style Guide Documentation — Lists   Ethereum org

@carlfairclough carlfairclough changed the title Created lists [fixes #767] Lists [fixes #767] Mar 18, 2020
Copy link
Member

@samajammin samajammin left a comment

Choose a reason for hiding this comment

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

Great work! Love the style guide preview. Commented on a few changes I'd like to see.

Also, what's the intended style for mobile, e.g. for rich lists? Should the width be 100%? The current width looks awkward:
Image 2020-03-18 at 9 49 41 AM

docs/.vuepress/theme/styles/lists.styl Show resolved Hide resolved
docs/.vuepress/components/ListCard.vue Outdated Show resolved Hide resolved
docs/developers/index.md Outdated Show resolved Hide resolved
docs/style-guide/lists/index.md Show resolved Hide resolved
docs/style-guide/lists/index.md Outdated Show resolved Hide resolved
docs/style-guide/lists/index.md Outdated Show resolved Hide resolved
docs/.vuepress/components/ListCard.vue Show resolved Hide resolved
docs/.vuepress/components/ListCard.vue Show resolved Hide resolved
content '↗️'

.list-new
box-shadow 0px 14px 66px rgba(0, 0, 0, 0.07), 0px 10px 17px rgba(0, 0, 0, 0.03), 0px 4px 7px rgba(0, 0, 0, 0.05)
Copy link
Member

Choose a reason for hiding this comment

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

Just a note - if we use these same shadows elsewhere (e.g. in Card components), we should consider setting these as variables.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I'll leave this here for now but I'm considering updating the card component to allow children, at which point I'll be able to remove this

@carlfairclough
Copy link
Contributor Author

@samajammin

Also, what's the intended style for mobile, e.g. for rich lists? Should the width be 100%? The current width looks awkward

This isn't an issue with the cards, it's an issue with the <pre> tags being wider than the viewport, which should be a separate PR. When tested on Safari mobile, I don't have the issue.

@carlfairclough
Copy link
Contributor Author

@samajammin all changes made apart from a couple. Added comments ☝️

Copy link
Member

@samajammin samajammin left a comment

Choose a reason for hiding this comment

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

Groovy 👍

Copy link
Member

@samajammin samajammin left a comment

Choose a reason for hiding this comment

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

Whoops, spoke to soon. Check out the list on the /developers page now, the arrows are showing up on the next line:
Image 2020-03-19 at 10 16 25 AM

This should be good to merge once this is fixed. Please be sure to QA your work locally / in the deploy preview!

@carlfairclough
Copy link
Contributor Author

@samajammin done! Not perfect (wide click target) but working

Added lists

removed file

removed duplicate rows
Removed list line-breaks

Removed internal prop, using utils.js

removed list-link-description

add target & rel props to external link items

Updated item title
@samajammin samajammin merged commit fe1755e into ethereum:dev Mar 19, 2020
@samajammin samajammin mentioned this pull request Mar 20, 2020
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.

2 participants