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

[Beta] Add EuiTour component #2766

Merged
merged 84 commits into from
Apr 14, 2020
Merged

[Beta] Add EuiTour component #2766

merged 84 commits into from
Apr 14, 2020

Conversation

ryankeairns
Copy link
Contributor

@ryankeairns ryankeairns commented Jan 16, 2020 •
edited by thompsongl
Loading

Closes #505

⬆️ Requirements and other considerations are listed in this issue.

Summary

The EuiTour component will lead users through a UI in a guided fashion. This Canvas tour prototype demonstrates one possible use case, though it should not be taken too literally.

What you'll see when you run this PR

euitour

Checklist

  • Check against all themes for compatibility in both light and dark modes
  • Checked in mobile
  • Checked in IE11 and Firefox
  • Props have proper autodocs
  • Added documentation examples
  • Added or updated jest tests
  • Checked for breaking changes and labeled appropriately
    - [ ] Checked for accessibility including keyboard-only and screenreader modes (follow-up PR)
  • A changelog entry exists and is marked appropriately

@ryankeairns
Copy link
Contributor Author

For those following along, Greg and I chatted yesterday. He is going to review the work done thus far and come up with a couple of options for handling state/context. Next, we'll solicit feedback from implementors regarding the proposed API options.

@ryankeairns
Copy link
Contributor Author

ryankeairns commented Feb 6, 2020 •
edited
Loading

@thompsongl @cqliu1 and I got together and discussed how we should proceed from here:

A couple of options were discussed regarding how to implement the component and we felt the most pragmatic approach would entail EUI to set a pattern for usage. This pattern approach puts the logic onus on the implementor/app team where the EUI component provides a robust API to manage each step.

Next milestone - EUI provide initial version of component

Our agreed upon goal is to get the EUI component ready to be PoC'd (according to the pattern approach) by the 7.7 feature freeze (3/24/2020). The initial PoC will be done in Kibana by the Canvas team (see next milestone).

The EUI work will include updating this PR. This work will include but is not limited to:

  • Re-naming the component(s) as necessary. Currently, the core component is named EuiTour, but there will be several instance per app (wrapping target elements), so something like EuiTourStep may make more sense, for example.
  • Cleaning up and adding props (according to the patterned approach)
  • Cleaning up the pass-thru interactions. For example, unlike a regular EuiPopover, this component will wrap existing components (e.g. buttons) which should retain their current click actions.
  • Updating the documentation to describe the pattern approach

Subsequent milestone - Canvas implements initial component as proof of concept

The Canvas team will implement the initial component in a proof of concept fashion, evaluate the API, and provide feedback during the 7.8 release. At that point, let's regroup and determine our next steps.

Future consideration - Expanding usage across Kibana

Further down the line, as more Kibana teams adopt this component, we may want to explore a higher level wrapper-style component (in Kibana) which allows teams to 'register' their tour and also allow admins to disable tours across an entire instance.

@kibanamachine

This comment has been minimized.

Sign in to view
@kibanamachine

This comment has been minimized.

Sign in to view
@kibanamachine

This comment has been minimized.

Sign in to view
@kibanamachine

This comment has been minimized.

Sign in to view
@kibanamachine

This comment has been minimized.

Sign in to view
@kibanamachine

This comment has been minimized.

Sign in to view
@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_2766/

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_2766/

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_2766/

@thompsongl
Copy link
Contributor

jenkins test this

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_2766/

@chandlerprall
Copy link
Contributor

Only thing left that stands out to me is if the anchor point moves, demonstrable by switching to the Props tab

static placement

@thompsongl
Copy link
Contributor

Only thing left that stands out to me is if the anchor point moves, demonstrable by switching to the Props tab

Good catch, and something we'll want to address. I haven't been able to think of an event-based way to track DOM position changes, though. I'm going to create a follow-up issue.

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_2766/

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_2766/

@ryankeairns
Copy link
Contributor Author

tenor-221693329

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_2766/

chandlerprall
chandlerprall approved these changes Apr 14, 2020
View reviewed changes
Copy link
Contributor

@chandlerprall chandlerprall left a comment

Choose a reason for hiding this comment

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

Looks great!

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_2766/

@thompsongl thompsongl merged commit 1a514a1 into elastic:master Apr 14, 2020
@thompsongl thompsongl mentioned this pull request May 8, 2020
Merged
dimitropoulos
dimitropoulos reviewed May 25, 2020
View reviewed changes
src-docs/src/views/tour/fullscreen.js
] = useEuiTour(demoTourSteps, tourConfig);

useEffect(() => {
console.log('Update', reducerState);
Copy link
Contributor

Choose a reason for hiding this comment

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

@thompsongl was it intentional to leave these console.log statements in here (there are two more that start with 'Updating localstorage')? I found it strange/surprising that it logs on production (well, on the EUI docs site, haha).

Copy link
Contributor

Choose a reason for hiding this comment

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

Haha nope. We should remove these.

Copy link
Contributor

Choose a reason for hiding this comment

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

luckily, I made the branch that fixes this last night :) #3513

dimitropoulos added a commit to dimitropoulos/eui that referenced this pull request May 26, 2020
@dimitropoulos
removes errant console.logs from EuiTour
fe100a0 
see: elastic#2766 (comment)
@dimitropoulos dimitropoulos mentioned this pull request May 26, 2020
Merged
chandlerprall pushed a commit that referenced this pull request May 26, 2020
@dimitropoulos
removes errant console.logs from EuiTour (#3513)
454c734 
see: #2766 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dimitropoulos dimitropoulos dimitropoulos left review comments

@thompsongl thompsongl thompsongl left review comments

@myasonik myasonik myasonik requested changes

@cchaos cchaos cchaos approved these changes

@snide snide snide approved these changes

@chandlerprall chandlerprall chandlerprall approved these changes

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

Successfully merging this pull request may close these issues.

EuiTour component
8 participants
@ryankeairns @kibanamachine @thompsongl @chandlerprall @myasonik @snide @cchaos @dimitropoulos