Skip to content
This repository has been archived by the owner on Dec 17, 2019. It is now read-only.

AEROGEAR-7665 mobile client creation view #34

Merged
merged 6 commits into from
Aug 8, 2018
Merged

AEROGEAR-7665 mobile client creation view #34

merged 6 commits into from
Aug 8, 2018

Conversation

wojta
Copy link
Contributor

@wojta wojta commented Aug 3, 2018 •
edited

https://issues.jboss.org/browse/AEROGEAR-7665

  • create mobile client form
  • make it modal dialog
  • add validation
  • prepare for backend connection
  • do the backend connection

creationclient

@wojta wojta mentioned this pull request Aug 3, 2018
Closed
4 tasks
@coveralls
Copy link

coveralls commented Aug 3, 2018 •
edited

Pull Request Test Coverage Report for Build 385

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 70.492%
Totals Coverage Status
Change from base Build 374: 0.0%
Covered Lines: 215
Relevant Lines: 305
💛 - Coveralls

@wojta wojta force-pushed the AEROGEAR-7665-mobile-app-creation branch 2 times, most recently from 96c1f02 to 2feeb0d Compare August 7, 2018 12:24
@wojta wojta changed the title [WIP] AEROGEAR-7665 mobile client creation view AEROGEAR-7665 mobile client creation view Aug 7, 2018
@wojta
Copy link
Contributor Author

wojta commented Aug 7, 2018

Now it's finalized and connected to the /api endpoint.

@wei-lee
Copy link
Contributor

wei-lee commented Aug 7, 2018

👀

@wei-lee
Copy link
Contributor

wei-lee commented Aug 7, 2018

@wojta actually, can you rebase and I will take a look

wei-lee
wei-lee reviewed Aug 7, 2018
View reviewed changes
ui/src/components/create_client/CreateClient.js Outdated
headers: {
"Content-Type": "application/json; charset=utf-8"
},
body: JSON.stringify(this.state.clientConfiguration)
Copy link
Contributor

Choose a reason for hiding this comment

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

we need to add credentials: "same-origin" option here as well

@wojta wojta force-pushed the AEROGEAR-7665-mobile-app-creation branch from 8eb9d82 to a4395e0 Compare August 7, 2018 16:18
wei-lee
wei-lee reviewed Aug 7, 2018
View reviewed changes
Gopkg.lock Outdated
branch = "master"
name = "github.com/aerogear/mobile-client-service"
packages = ["pkg/apis/aerogear/v1alpha1","pkg/config","pkg/mobile","pkg/stub","pkg/web"]
revision = "55a868cebcad0e79f61fd77842833ab0866ffc8d"
Copy link
Contributor

Choose a reason for hiding this comment

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

this should not be here.

wei-lee
wei-lee reviewed Aug 7, 2018
View reviewed changes
Gopkg.lock Outdated
"pkg/generators/rules",
"pkg/util/sets"
]
packages = ["cmd/openapi-gen/args","pkg/common","pkg/generators","pkg/generators/rules","pkg/util/sets"]
revision = "e3762e86a74c878ffed47484592986685639c2cd"

[solve-meta]
Copy link
Contributor

Choose a reason for hiding this comment

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

are you using dep? Why the format of this file changed so much?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe, I shouldn't be changing that file at all, I'll try to remove the changes.

wei-lee
wei-lee reviewed Aug 7, 2018
View reviewed changes
ui/src/components/overview/Overview.js Outdated
@@ -67,6 +81,8 @@ class Overview extends Component {
mobileClients={mobileClients}
mobileServiceInstances={mobileServiceInstances}
mobileClientBuilds={mobileClientBuilds}></MobileClientOverviewList>
<MobileClientOverviewList mobileClients={this.state.mobileClients}></MobileClientOverviewList>
Copy link
Contributor

Choose a reason for hiding this comment

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

this line should not be here. Looks like a merge error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes

wei-lee
wei-lee reviewed Aug 7, 2018
View reviewed changes
package-lock.json Outdated
@@ -0,0 +1,3 @@
{
"lockfileVersion": 1
}
Copy link
Contributor

Choose a reason for hiding this comment

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

is this file actually required?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, probably caused by accidentally running npm in parent directory

wei-lee
wei-lee suggested changes Aug 7, 2018
View reviewed changes
Copy link
Contributor

@wei-lee wei-lee left a comment

Choose a reason for hiding this comment

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

See my comments.

Overall looks good. There are a few small issues with the UI:

  1. in the first step, the row of icons are not centered on the page
  2. on the last step, there are 3 buttons, but only close actually works.

It will be great if we can address them. But I am ok to merge this one first (once the blocker issues are fixed) and then address them in later PRs.

@wojta wojta force-pushed the AEROGEAR-7665-mobile-app-creation branch 2 times, most recently from ff945e7 to 4f2a6fa Compare August 8, 2018 10:35
wei-lee
wei-lee reviewed Aug 8, 2018
View reviewed changes
Gopkg.lock Outdated

[solve-meta]
analyzer-name = "dep"
analyzer-version = 1
inputs-digest = "4e9ea47f087096aad1fb93fb20b3cc4221ad565f10c6a0ae8ec4820fd4eb2f58"
inputs-digest = "f7e9fafd1a7bbc8e37ad45c4d4de5dc70845f9a32b2b1c98a214af158f163ed4"
solver-name = "gps-cdcl"
solver-version = 1
Copy link
Contributor

Choose a reason for hiding this comment

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

@wojta should this file be changed at all in this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no :D I thought that it didn't, I checked it out from master... weird

@wojta wojta force-pushed the AEROGEAR-7665-mobile-app-creation branch from 4f2a6fa to cabab20 Compare August 8, 2018 13:53
@wojta wojta force-pushed the AEROGEAR-7665-mobile-app-creation branch from cabab20 to e472e30 Compare August 8, 2018 13:54
@wei-lee wei-lee merged commit 3d42b28 into master Aug 8, 2018
@wei-lee wei-lee deleted the AEROGEAR-7665-mobile-app-creation branch August 8, 2018 14:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@wei-lee wei-lee wei-lee approved these changes

@sedroche sedroche Awaiting requested review from sedroche

@ciaranRoche ciaranRoche Awaiting requested review from ciaranRoche

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@wojta @coveralls @wei-lee