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

feat(components): tour component #14952

Merged
merged 1 commit into from
Dec 27, 2023
Merged

Conversation

Fuphoenixes
Copy link
Contributor

@Fuphoenixes Fuphoenixes commented Nov 25, 2023

Please make sure these boxes are checked before submitting your PR, thank you!

  • Make sure you follow contributing guide English | (中文 | Español | Français).
  • Make sure you are merging your commits to dev branch.
  • Add some descriptions and refer to relative issues for your PR.

Description

🤖[deprecated] Generated by Copilot at 3aa9d89

This pull request adds a new Tour component to the element-plus library, which allows users to create guided tours for their products. The component consists of a main Tour component and three sub-components: TourStep, TourMask and TourSteps. The pull request also includes the documentation, examples and tests for the component, as well as the necessary changes to the components index file and the component sidebar menu.

Related Issue

Fixes #___.

Explanation of Changes

🤖[deprecated] Generated by Copilot at 3aa9d89

  • Add a new Tour component to the components package, which guides users through a product with a series of steps (link, link, link, link)
  • Implement the Tour component using sub-components for the mask, the step and the steps, which handle the masking effect, the guide card and the steps rendering respectively (link, link, link, link, link)
  • Provide utility functions and types for the Tour component logic, such as useTarget, tourKey, isInViewPort, isSameProps, isSameSteps and getNormalizedProps (link)
  • Use the withInstall and withNoopInstall helpers to enable global and local registration of the Tour component and its sub-component TourStep (link)
  • Use the ElTooltip, ElButton and ElIcon components to enhance the Tour component UI and functionality (link, link)
  • Use the useLockscreen hook to prevent scrolling when the mask is visible (link)
  • Add unit tests for the Tour component using the vitest framework (link)
  • Add documentation for the Tour component, which includes the basic usage, examples, API, slots and events (link, link)
  • Add examples for the Tour component, which show how to use the component with different props and slots, such as mask, placement, type and indicator (link, link, link, link, link)

Copy link

👋 @Fuphoenixes, thank you for contributing element-plus.

  • You can comment with /label Components:[component_name] to add a label for which component you are working on.
  • You may join our Discord for staying tuned.

Copy link

Hello @Fuphoenixes, thank you for contributing to element-plus, please see our guideline to see how to make contribution

Copy link

github-actions bot commented Nov 25, 2023

Copy link

github-actions bot commented Nov 25, 2023

🧪 Playground Preview: https://element-plus.run/?pr=14952
Please comment the example via this playground if needed.

@Fuphoenixes
Copy link
Contributor Author

tour_zh-CN.md
这是该组件的中文文档的md文件

@Fuphoenixes
Copy link
Contributor Author

Fuphoenixes commented Nov 26, 2023

🧪 Playground Preview: https://element-plus.run/?pr=14952 Please comment the example via this playground if needed.

link

@Fuphoenixes
Copy link
Contributor Author

@btea do we need this component?

@btea
Copy link
Collaborator

btea commented Dec 4, 2023

cc @element-plus/backers

@kooriookami
Copy link
Member

Wow, it looks great. I tried it simply and found a problem. When using mask, the buttons behind the mask shouldn't be clickable.

@Fuphoenixes
Copy link
Contributor Author

Wow, it looks great. I tried it simply and found a problem. When using mask, the buttons behind the mask shouldn't be clickable.

thanks, i have already resolved it.

@Fuphoenixes
Copy link
Contributor Author

sorry, I changed my git account, so I force-pushed a few more times

@kooriookami
Copy link
Member

There're some problems:

  1. Focus on popup and press esc, the popup disappeared.
    image
  2. When popup has no space, click the begin tour button, it shows in the wrong place.
    image
    image

Copy link
Member

@ryuhangyeong ryuhangyeong left a comment

Choose a reason for hiding this comment

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

Opinion

  1. There may be cases where it cannot be used properly in a mobile environment.

    image
  2. focus trap not working

  3. button is styled as if it can't go to the next step, even though it could. (It seems to be a bug)

    image

@Fuphoenixes
Copy link
Contributor Author

There're some problems:

  1. Focus on popup and press esc, the popup disappeared.
    image
  2. When popup has no space, click the begin tour button, it shows in the wrong place.
    image
    image

Opinion

  1. There may be cases where it cannot be used properly in a mobile environment.
    image
  2. focus trap not working
  3. button is styled as if it can't go to the next step, even though it could. (It seems to be a bug)
    image

I refactored it and then resolved these issues.

@Fuphoenixes
Copy link
Contributor Author

image Display exceptions outside of the doc.

done

docs/en-US/component/tour.md Outdated Show resolved Hide resolved
docs/en-US/component/tour.md Outdated Show resolved Hide resolved
docs/en-US/component/tour.md Show resolved Hide resolved
@Fuphoenixes
Copy link
Contributor Author

@btea resolved

@kooriookami
Copy link
Member

I think if no other bugs, it can be merged.

@FrontEndDog
Copy link
Member

Wow, it looks great. I tried it simply and found a problem. When using mask, the buttons behind the mask shouldn't be clickable.

This issue has not been resolved.

docs/en-US/component/tour.md Outdated Show resolved Hide resolved
docs/en-US/component/tour.md Outdated Show resolved Hide resolved
docs/en-US/component/tour.md Outdated Show resolved Hide resolved
@Fuphoenixes
Copy link
Contributor Author

Wow, it looks great. I tried it simply and found a problem. When using mask, the buttons behind the mask shouldn't be clickable.

This issue has not been resolved.

Hasn't this issue been resolved? Does it mean that the buttons in this blank area cannot be clicked?
image

@Fuphoenixes
Copy link
Contributor Author

I think this should allow for clicks, which might be useful in some cases.

@FrontEndDog
Copy link
Member

I think this should allow for clicks, which might be useful in some cases.

I think it's more reasonable not to be clickable.
For example, If these buttons are clicked, it will cause the page to jump, it will interrupt the guidance process.

@Fuphoenixes
Copy link
Contributor Author

I think this should allow for clicks, which might be useful in some cases.

I think it's more reasonable not to be clickable. For example, If these buttons are clicked, it will cause the page to jump, it will interrupt the guidance process.

How about adding a configuration and letting the user decide? Not clickable by default?

@FrontEndDog
Copy link
Member

I think this should allow for clicks, which might be useful in some cases.

I think it's more reasonable not to be clickable. For example, If these buttons are clicked, it will cause the page to jump, it will interrupt the guidance process.

How about adding a configuration and letting the user decide? Not clickable by default?

Yes, This is better.

@Fuphoenixes
Copy link
Contributor Author

Okay, I have added it

Copy link
Member

@FrontEndDog FrontEndDog left a comment

Choose a reason for hiding this comment

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

Thanks, It is great!
Merry Christmas!

docs/en-US/component/tour.md Outdated Show resolved Hide resolved
@kooriookami kooriookami merged commit 2fd7ba8 into element-plus:dev Dec 27, 2023
8 checks passed
@element-bot element-bot mentioned this pull request Jan 10, 2024
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants