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

Refactor v3 #379

Merged
merged 57 commits into from
Oct 16, 2018
Merged

Refactor v3 #379

merged 57 commits into from
Oct 16, 2018

Conversation

edewit
Copy link
Member

@edewit edewit commented Oct 3, 2018

Goal of this refactor is to make the library more extendable for the client. In order to do that we've introduced a way to create custom "steps". To do so one will have to extend LauncherStep and implement:

restoreModel(model: any): void 

This is used to save the state in the URL for the authentication redirects.

Usage also changes instead of 'just' adding <f8launcher one needs to add the steps like this:

  <f8launcher (onCancel)="cancel()"> <!-- can remove this if you don't want scrolling wizard layout -->
    <div class="f8launcher-container_nav">
      <f8launcher-step-indicator #stepIndicator></f8launcher-step-indicator>
    </div>
    <div class="f8launcher-container_main">
      <f8launcher-targetenvironment-createapp-step id="TargetEnvironment"> <!-- standalone reusable --> 
      </f8launcher-targetenvironment-createapp-step>
      ...
   </f8launcher>

Also new in this version is the ability to customize the 'next buttons' with a section like this:

<f8launcher>
...
  <div class="f8launcher-container_completed-footer">
    <button class="btn btn-primary btn-lg" (click)="complete()">
      View New Application
    </button>
  </div>
</f8launcher>

This will also aid in tailored user experience

BREAKING CHANGE: new client API
fixes: #369

@ghost ghost assigned edewit Oct 3, 2018
@ghost ghost added the in progress label Oct 3, 2018
@bartoszmajsak
Copy link
Contributor

bartoszmajsak commented Oct 4, 2018

BREAKING CHANGE: new client API

@edewit can you point me to those breaking changes so we can understand as impact?

@edewit
Copy link
Member Author

edewit commented Oct 5, 2018

@bartoszmajsak instead of using <f8launcher></f8launcher> one needs to add the steps that one wants to use for an example of this have a look at the demo application. Also the API to send the data to the backend has changed there is no summary object anymore but a projectile that has to HttpParams function one can use

Copy link
Contributor

@ia3andy ia3andy left a comment

Choose a reason for hiding this comment

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

Could you squash or rename commits?

@edewit
Copy link
Member Author

edewit commented Oct 8, 2018

@ia3andy I'll squash on merge?

Copy link
Contributor

@ia3andy ia3andy left a comment

Choose a reason for hiding this comment

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

I hope I didn't miss something, but it looks good to me ^^

@@ -4,4 +4,5 @@ export class DependencyCheck {
projectName: string;
projectVersion: string;
spacePath: string;
targetEnvironment: string;
Copy link
Collaborator

Choose a reason for hiding this comment

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

targetEnviroment is used by the only standalone launcher. So, can you make as an optional attribute?

@vikram-raj
Copy link
Collaborator

@edewit import flow is not working for the demo app. I am getting an error that the repository does not exist.

@edewit
Copy link
Member Author

edewit commented Oct 12, 2018

@vikram-raj import is fixed with 02d640a

@edewit
Copy link
Member Author

edewit commented Oct 12, 2018

full doc is available https://hackmd.io/wR4tP6KqQZOWqM1Tk3_tUQ?view

@vikram-raj
Copy link
Collaborator

@edewit Don't you think we don't need this variable (nextButtons) in ngx-launcher? I introduce this varibale to handle feature flag for view pipeline and Edit Application in Web IDE button from fabric8-ui.

@edewit
Copy link
Member Author

edewit commented Oct 15, 2018

@edewit Don't you think we don't need this variable (nextButtons) in ngx-launcher? I introduce this varibale to handle feature flag for view pipeline and Edit Application in Web IDE button from fabric8-ui.

With the new setup these buttons can be handled by the client (fabric8-ui) no need to have such a switch on the ngx-launcher side

@vikram-raj
Copy link
Collaborator

@edewit So, can you please remove this variable?

@edewit
Copy link
Member Author

edewit commented Oct 15, 2018

@vikram-raj oops sorry I thought I did

Copy link
Collaborator

@vikram-raj vikram-raj left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks, @edewit for addressing all my comments.

@edewit edewit merged commit be52635 into master Oct 16, 2018
@ghost ghost removed the in progress label Oct 16, 2018
@edewit edewit deleted the refactor-v3 branch October 16, 2018 09:57
@fabric8cd
Copy link

🎉 This PR is included in version 3.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor: Split ngx-launcher in multiple components accessible from client app
5 participants