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

Making next steps/results messaging consistent #2184

Merged
merged 1 commit into from Sep 29, 2017
Merged

Making next steps/results messaging consistent #2184

merged 1 commit into from Sep 29, 2017

Conversation

rhamilto
Copy link
Member

@rhamilto rhamilto commented Sep 28, 2017

To Do:

  • Update styling of wizard contents once comps are finalized per @ncameronbritt and @jennyhaines

  • Implement changes to remove mega alert and show simple bulleted list like create-from-builder-results.html for select from project (wizard) and import (next steps and wizard)

  • Display webhook info when appropriate inside wizard results (currently only shows on standalone next steps)

  • Show proper type of imported item in import wizard results (non-templates are generically called "YAML / JSON")

  • Remove import toast notifications (currently needed for standalone import next steps as it redirects to overview)

  • Improve display of provision call failure message
    screen shot 2017-10-02 at 3 25 21 pm
    screen shot 2017-10-02 at 3 29 01 pm
    screen shot 2017-10-02 at 3 29 14 pm
    screen shot 2017-10-03 at 11 27 16 am

  • Add appropriate actions when failure occurs

Screenshots:

screen shot 2017-09-28 at 2 44 17 pm

screen shot 2017-09-28 at 3 09 26 pm

screen shot 2017-09-28 at 3 16 47 pm

screen shot 2017-09-28 at 3 17 19 pm

screen shot 2017-09-28 at 12 06 05 pm

screen shot 2017-09-28 at 12 10 22 pm

screen shot 2017-09-28 at 12 11 35 pm

screen shot 2017-09-29 at 2 33 57 pm

screen shot 2017-09-29 at 2 36 59 pm

screen shot 2017-09-29 at 2 53 40 pm

</h1>
</div>
</div>
<!-- if success and the user refreshes the next steps page -->
Copy link
Member

Choose a reason for hiding this comment

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

Technically this happens even if the user refreshed the next steps page after a failure. We might want a more neutral message.

@spadgett
Copy link
Member

Display webhook info when appropriate inside wizard results (currently only shows on standalone next steps)

I think the problem here is that we're just not setting the webhook when not using advanced options. This is arguably just a bug.

@spadgett
Copy link
Member

Implement changes to remove mega alert and show simple bulleted list like create-from-builder-results.html for select from project (wizard) and import (next steps and wizard)

I wonder if we shouldn't just change the styling of the tasks component and remove it from the overview.

@ncameronbritt
Copy link

Since we have the different webhook options (bitbucket, gitlab), is that something we would want to let users set up as part of this flow, and then show the results on that last step? I'm not sure how common it is that a user would want to set those up.

@jhaines was working on restyling the tasks component, but that's on hold because I was wanting to figure out what a user should do after a partial successes/failure, and what's important for the user to know to take that next step.

@rhamilto rhamilto changed the title [WIP] Making next steps/results messaging consistent Making next steps/results messaging consistent Sep 29, 2017
@rhamilto
Copy link
Member Author

@spadgett, I think this is ready for review/merge with the understanding we'll tackle the remaining to-dos as follow-ons.

@spadgett
Copy link
Member

Since we have the different webhook options (bitbucket, gitlab), is that something we would want to let users set up as part of this flow, and then show the results on that last step? I'm not sure how common it is that a user would want to set those up.

We should probably be consistent in what we set up by default in the dialog vs advanced. There is a checkbox for it in advanced options.

If you are using GitHub, Bitbucket, etc., you probably want them. But if you're not using GitHub and we set up a GitHub webhook for you, that's not helpful. I don't know if we can reliably detect from the Git URL which ones to create.

Copy link
Member

@spadgett spadgett left a comment

Choose a reason for hiding this comment

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

Thanks @rhamilto, just one comment


<div ng-if="$ctrl.createdBuildConfig" class="col-md-6">
<h2>Making code changes</h2>
<div ng-controller="TasksController" ng-if="$ctrl.createdBuildConfig">
Copy link
Member

Choose a reason for hiding this comment

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

Remove ng-controller here

@rhamilto
Copy link
Member Author

Comment addressed.

@rhamilto
Copy link
Member Author

We should probably be consistent in what we set up by default in the dialog vs advanced. There is a checkbox for it in advanced options.

And that checkbox is enabled by default. So if you use the wizard, you get no webhook. But if you used advanced with the default, you do. Seems a bit confusing?

@ncameronbritt
Copy link

Agree with your comments above re webhooks @spadgett. If it's useful, maybe down the line we can look at adding a step to the wizard or including it in the builder flow some other way.

@spadgett
Copy link
Member

[merge]

@openshift-bot
Copy link

Evaluated for origin web console merge up to 6f50f95

@openshift-bot
Copy link

openshift-bot commented Sep 29, 2017

Origin Web Console Merge Results: SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_request_origin_web_console/297/) (Base Commit: 8498bca) (PR Branch Commit: 6f50f95)

@openshift-bot openshift-bot merged commit 7f03cb7 into openshift:master Sep 29, 2017
@rhamilto rhamilto deleted the results branch September 29, 2017 20:22
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

4 participants