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

Downstream changes from Explorer UX upgrades #48

Merged
merged 9 commits into from
Mar 11, 2019
Merged

Conversation

sarathms
Copy link
Contributor

@sarathms sarathms commented Mar 8, 2019

OONI Explorer is going through a major UX overhaul. There are some elements of the design system which are being redesigned. These will eventually land in other applications using them.

@elioqoshi
Copy link
Contributor

Use camel-case on buttons

Just little nitpicking that it's not camel-case but simply Title Case

@sarathms sarathms requested a review from hellais March 11, 2019 13:22
maxWidth: [768, 1024, 1280]
}

export default Container
Copy link
Member

Choose a reason for hiding this comment

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

This is cool. I like!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I put very little thought into what the values should be. They sound reasonable, we need to make sure they look appropriate too.

"jest-styled-components": "^6.2.0",
"react-test-renderer": "^16.5.2"
"jest": "^24.3.1",
"jest-styled-components": "^6.3.1",
Copy link
Member

Choose a reason for hiding this comment

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

@sarathms let's not forget to file a ticket about jest-styled-components and it blocking on us upgrading to styled-components v4.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Filed in #49

@hellais
Copy link
Member

hellais commented Mar 11, 2019

I left a couple of comments on this PR, but it looks good to me and I would say it can be merged.

@sarathms do you need permissions to push to npm or do you have those already?

@sarathms
Copy link
Contributor Author

I have published this package on npm before. I will go ahead.

@sarathms sarathms merged commit d9b46fd into master Mar 11, 2019
@sarathms sarathms deleted the fix/from-explorer-ux branch July 9, 2019 14:45
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.

None yet

3 participants