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.

Sign up for GitHub

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

Jump to bottom

Getting Started Tour #6011

Merged
merged 1 commit into from Jul 29, 2020
Merged

Getting Started Tour #6011

merged 1 commit into from Jul 29, 2020

Conversation

sahil143
Copy link
Contributor

@sahil143 sahil143 commented Jul 16, 2020 •
edited

Story: https://issues.redhat.com/browse/ODC-4262, https://issues.redhat.com/browse/ODC-4090

Analysis: Missing Getting Started Tour Experience

Solution: Added the TourComponent for Getting Started Tour.

ScreenShot:
tour

Screenshot 2020-07-20 at 7 33 14 PM

Screenshot 2020-07-20 at 7 34 00 PM

Screenshot 2020-07-20 at 7 33 42 PM

Screenshot 2020-07-20 at 7 33 25 PM

UnitTests
[TODO]

Browser:
[x] Chrome

@openshift-ci-robot openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jul 16, 2020
@openshift-ci-robot
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci-robot openshift-ci-robot added the component/core Related to console core functionality label Jul 16, 2020
@openshift-ci-robot openshift-ci-robot added component/dev-console Related to dev-console component/sdk Related to console-plugin-sdk component/shared Related to console-shared labels Jul 16, 2020
@sahil143 sahil143 changed the title Getting Started Tour [WIP] Getting Started Tour Jul 16, 2020
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 20, 2020
@sahil143 sahil143 marked this pull request as ready for review July 20, 2020 14:06
@sahil143 sahil143 changed the title [WIP] Getting Started Tour Getting Started Tour Jul 20, 2020
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 20, 2020
@rohitkrai03
Copy link
Contributor

/assign

rohitkrai03
rohitkrai03 suggested changes Jul 21, 2020
View reviewed changes
frontend/packages/console-app/src/components/tour/CloseModal.tsx Outdated Show resolved Hide resolved
frontend/packages/console-app/src/components/tour/Spotlight.scss Outdated Show resolved Hide resolved
frontend/packages/console-app/src/components/tour/Spotlight.tsx Outdated Show resolved Hide resolved
frontend/packages/console-app/src/components/tour/Spotlight.tsx Outdated Show resolved Hide resolved
frontend/packages/console-app/src/components/tour/Spotlight.scss Outdated Show resolved Hide resolved
frontend/packages/console-app/src/components/tour/TourStepComponent.scss Outdated Show resolved Hide resolved
frontend/packages/console-app/src/components/tour/TourStepComponent.tsx Outdated Show resolved Hide resolved
frontend/packages/console-app/src/components/tour/steps/StepHeader.tsx Show resolved Hide resolved
frontend/packages/console-app/src/components/tour/tour-context.ts Show resolved Hide resolved
frontend/packages/console-app/src/components/tour/tour-context.ts Outdated Show resolved Hide resolved
christianvogt
christianvogt reviewed Jul 22, 2020
View reviewed changes
frontend/packages/console-app/src/components/tour/Spotlight.scss Outdated Show resolved Hide resolved
frontend/packages/console-app/src/components/tour/Spotlight.scss Outdated Show resolved Hide resolved
frontend/packages/console-app/src/components/tour/Spotlight.tsx Show resolved Hide resolved
frontend/packages/console-app/src/components/tour/Spotlight.tsx Show resolved Hide resolved
frontend/packages/console-app/src/components/tour/Spotlight.tsx Outdated Show resolved Hide resolved
frontend/packages/console-app/src/components/tour/steps/StepBadge.tsx Outdated Show resolved Hide resolved
frontend/packages/console-app/src/components/tour/steps/StepFooter.tsx Outdated Show resolved Hide resolved
frontend/packages/console-app/src/components/tour/tour-context.ts Show resolved Hide resolved
frontend/packages/console-app/src/components/tour/tour-context.ts Show resolved Hide resolved
frontend/packages/console-app/src/components/tour/type.ts Outdated Show resolved Hide resolved
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 22, 2020
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 22, 2020
@sahil143
Copy link
Contributor Author

@rohitkrai03 @christianvogt Ready for review again.

rohitkrai03
rohitkrai03 reviewed Jul 22, 2020
View reviewed changes
Copy link
Contributor

@rohitkrai03 rohitkrai03 left a comment •
edited

Choose a reason for hiding this comment

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

@sahil143 Please squash your commits. Apart from the highlight border color being blue and dummy texts in the tours, code looks good to me. I'll give it another run tomorrow.

@bkrikori
Copy link

Help menu - order of importance
@sahil143 Here is the design for the help menu. In the spotlight on elements, please also change the blue to #0066CC or $pf-color-blue-400

@christianvogt
Copy link
Contributor

@sahil143 after existing the tour early, if I then use the menu to start the tour again I'm brought to the are you sure you want to leave model.

@beaumorley
Copy link

@sahil143 A link to a GDoc with final copy is attached to the DTUX story.

rohitkrai03
rohitkrai03 approved these changes Jul 27, 2020
View reviewed changes
Copy link
Contributor

@rohitkrai03 rohitkrai03 left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 27, 2020
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

3 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@sahil143
Copy link
Contributor Author

/hold

integration tests are failing 😞

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 27, 2020
@sahil143
Copy link
Contributor Author

/retest

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jul 27, 2020
@sahil143 sahil143 force-pushed the product-tour branch 2 times, most recently from 3852a70 to b0910d5 Compare July 28, 2020 09:18
@rohitkrai03
Copy link
Contributor

/retest

rohitkrai03
rohitkrai03 approved these changes Jul 28, 2020
View reviewed changes
Copy link
Contributor

@rohitkrai03 rohitkrai03 left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 28, 2020
@sahil143
Copy link
Contributor Author

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 29, 2020
@sahil143
Guided Tour create components for the getting started experience
40fd4fd 
fix lint and code error

fix ContextProvider extension usage issue with guided tours

fix type for guided tour step and css for modal footer

GuidedTour: change flags type for step to array, add additional check for element is present in the dom or not

Guided Tour: make TourStepcomponent a pure component, created StepComponent to control actions for the tour
created portal for the spotlight

Guided Tour: store completion of tour to local storage

Guided Tour: add unit tests for components and utils

revert change

Guided Tour: add comment for not using getActivePerspective in context value hook

Guided Tour: set closeTour as false when starting the tour

Guided Tour: Add text for the steps and create component

remove guided-tours from utils

Guided Tour: update styles for spotlight

remove p tag from finish modal

fix rerendering of the components

fix integration tests
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jul 29, 2020
@invincibleJai
Copy link
Member

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 29, 2020
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: christianvogt, invincibleJai, rohitkrai03, sahil143

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-robot openshift-merge-robot merged commit 0b61e9e into openshift:master Jul 29, 2020
@glekner
Copy link
Contributor

glekner commented Jul 29, 2020

@spadgett
Looks like this PR broke master for me, did you test these changes with a ContextProvider extension?
I'm running with Kubevirt installed, which has a ContextProvider extension and getting the following errors:

Screen Shot 2020-07-29 at 14 31 29

Screen Shot 2020-07-29 at 14 31 40

@spadgett spadgett added this to the v4.6 milestone Jul 30, 2020
@jerolimov jerolimov mentioned this pull request Jul 30, 2020
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@christianvogt christianvogt christianvogt left review comments

@rohitkrai03 rohitkrai03 rohitkrai03 approved these changes

Assignees

@invincibleJai invincibleJai

@rohitkrai03 rohitkrai03

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. component/core Related to console core functionality component/dev-console Related to dev-console component/sdk Related to console-plugin-sdk component/shared Related to console-shared lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Milestone
v4.6
Development

Successfully merging this pull request may close these issues.

None yet