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

App Center: connect to installed repos #694

Merged
merged 6 commits into from Apr 14, 2019

Conversation

Projects
None yet
2 participants
@sohkai
Copy link
Member

sohkai commented Apr 13, 2019

Requires aragon/aragon.js#268.

Cherry-picks d292440 from #691 (for now).

Refactors the App Center to use "repos" as their data structure representation, and connect to web3 data coming from @aragon/wrapper's installedRepos.

@sohkai sohkai requested review from bpierre and 2color Apr 13, 2019

@bpierre
Copy link
Member

bpierre left a comment

LGTM!

I think there is an issue with the transition of the versions panel, but I can look into it separately.

@@ -56,7 +56,7 @@ const AppIconContent = ({ app, size, src }) => {
const iconUrl = appIconUrl(app)
return (
// Tries to load the app icon while displaying the default one.
<RemoteImage size={size} src={iconUrl}>
<RemoteImage src={iconUrl}>

This comment has been minimized.

Copy link
@bpierre
@@ -15,9 +16,12 @@ export function compose(...funcs) {

// Get the icon URL of an app
export function appIconUrl(app) {

This comment has been minimized.

Copy link
@bpierre

bpierre Apr 13, 2019

Member

This should probably be renamed repoIconUrl(repo) in the future. And maybe we could keep AppIcon (not renaming it RepoIcon), since it’s a component and not related to the data.

This comment has been minimized.

Copy link
@sohkai

sohkai Apr 14, 2019

Author Member

I've been thinking of modifying this to be more generic with #691 to be iconUrlFromBase() or something.

</h1>
{canUpgrade && (
<div
<RemoteImage src={iconUrl}>

This comment has been minimized.

Copy link
@bpierre

bpierre Apr 13, 2019

Member

We should use AppIcon here, so we get the radius applied too (I can look into it).

height: 80px;
`}
/>
<RemoteImage src={iconUrl}>

This comment has been minimized.

Copy link
@bpierre

bpierre Apr 13, 2019

Member

Same here about AppIcon.

sohkai added some commits Apr 12, 2019

@sohkai sohkai force-pushed the connect-installed-repos branch 2 times, most recently from 4ec751f to 2add5ab Apr 14, 2019

@sohkai sohkai changed the base branch from restart-app-worker to app-center Apr 14, 2019

@sohkai sohkai merged commit 954b448 into app-center Apr 14, 2019

1 of 3 checks passed

License Compliance FOSSA is analyzing this commit
Details
continuous-integration/travis-ci/push The Travis CI build is in progress
Details
license/cla Contributor License Agreement is signed.
Details

@sohkai sohkai deleted the connect-installed-repos branch Apr 14, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.