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

Tour name is not being added as data-shepherd-active-tour attribute #1188

Closed
zefj opened this issue Oct 7, 2020 · 13 comments
Closed

Tour name is not being added as data-shepherd-active-tour attribute #1188

zefj opened this issue Oct 7, 2020 · 13 comments

Comments

@zefj
Copy link
Contributor

zefj commented Oct 7, 2020

Hey, found a discrepancy between the docs and the library behaviour.

tourName: An optional "name" for the tour. This will be appended to the the tour's dynamically generated id property -- which is also set on the body element as the data-shepherd-active-tour attribute whenever the tour becomes active.

This property is not being added. For some reason it was removed in 0594278. I think it's a useful feature though, so I suggest what follows:

  1. Bring back the data-shepherd-active-tour attribute on document.body
  2. Make the step id appear as data-shepherd-active-step when the step is shown

With this improvement, the toured application could apply CSS rules to elements based on active tour and step, without the need for explicit javascript logic. This is something I could really use right now.

I could prepare a PR, what do you say? @rwwagner90

@zefj
Copy link
Contributor Author

zefj commented Oct 7, 2020

Regarding 2., I have noticed tour id and step id are designed rather poorly:

  • Tour id is ${TourOptions.tourName}-${uuid} if TourOptions.tourName is defined, or tour-${uuid}
  • Step id is StepOptions.id if StepOptions.id is defined, or step-${uuid}

So step id is configurable, but tour id is not. However, a tour gets a name, which is then used when generating tour id... Kinda convoluted if you ask me.

We could make this more coherent. What do you say we introduce TourOptions.id, and make the tour id behave like step id (TourOptions.id or tour-${uuid})? TourOptions.tourName could then be deprecated and removed in future version as obsolete.

@stale
Copy link

stale bot commented Nov 7, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Nov 7, 2020
@stale stale bot closed this as completed Nov 15, 2020
@stale stale bot removed the wontfix label Nov 16, 2020
@stale
Copy link

stale bot commented Dec 19, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale
Copy link

stale bot commented Feb 27, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale
Copy link

stale bot commented Jun 9, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Jun 9, 2021
@stale
Copy link

stale bot commented Jul 21, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Jul 21, 2021
@Steezli
Copy link
Contributor

Steezli commented Aug 3, 2022

No discussion for over a year, closing this issue.

@RobbieTheWagner
Copy link
Member

@EmNicholson93 we should make sure our docs are correct here. This issue is saying we are not setting data-shepherd-active-tour like we say we are in the docs. Can we confirm?

@Steezli
Copy link
Contributor

Steezli commented Aug 3, 2022

@rwwagner90 The code and tests for this feature were removed as part of launching v10.0.1. So assuming that was intentional, I can pull mentions of tourName from all the docs?

@RobbieTheWagner
Copy link
Member

@EmNicholson93 Do you happen to know the PR or commit where it was all removed? I cannot recall why we removed it, but if we definitely want it gone, then yeah the fix here would be to remove all this stuff from the docs.

@Steezli
Copy link
Contributor

Steezli commented Aug 4, 2022

@rwwagner90
Here is the PR when the _addBodyAttrs and _removeBodyAttrs were removed

@RobbieTheWagner
Copy link
Member

@EmNicholson93 yes, it appears we did remove all that. I think to close this ticket then we should remove all that from the docs.

@Steezli
Copy link
Contributor

Steezli commented Aug 18, 2022

#2022

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

No branches or pull requests

3 participants