shipshapecode / shepherd

Guide your users through a tour of your app
https://shepherdjs.dev
Other
13.01k stars 648 forks source link

arrow breaks layout after upgrading to 6.0 (and highlighting no longer works) #661

Closed vesper8 closed 4 years ago

vesper8 commented 4 years ago

I am trying to upgrade to 6.0 and running into two issues.. well the first issue is that the highlighting has completely stopped working.. as you'll see in the screenshots below.

But the odder issue is that the arrow (I'm using the theme from https://shepherdjs.dev/) breaks the layout of my nav menu when I visit steps that target nav items

This is how it looked on 5.0:

Screen Shot 2019-11-05 at 12 01 23 AM

Highlighting works beautifully, arrow doesn't break layout

Then on 6.0:

Screen Shot 2019-11-05 at 12 46 22 AM

Highlighting no longer working, arrow has somehow been inserted inside the nav and caused the caret to flow to the next line

I am using a custom overlay by the way.. on 5.0 the highlighting was just working despite my custom overlay. The reason I use a custom overlay is because I'm using a "hack" to allow the tour to be cancelled when clicking on the overlay.. something I found on one of the issues.. because I believe you still don't want to implement this as part of the package? Or has this now changed with 6.0

In any case, is the highlight somehow related to the use of the built-in overlay now?

Regarding the anomaly, even after I cancel/complete the tour, the anomaly remains:

Screen Shot 2019-11-05 at 12 08 12 AM

Upon inspecting the nav item with the debugger, I noticed that this was inserted, and left behind after the tour was done:

<div class="shepherd-target-marker"><div class="shepherd-marker-dot" style="top: 0px; left: 0px;"></div></div>
vesper8 commented 4 years ago

Perhaps of less importance but I'd also like to note that previously the arrow aligned nicely.. whereas now the arrow is on top of the element it's supposed to be pointing too. Ideally the popup should be a little more spaced away from the item so that the arrow points TO the tether and isn't on top of it

vesper8 commented 4 years ago

I just edited my first post with more details

vesper8 commented 4 years ago

Upon further testing, it seems that the layout breakage is not related to the arrow because even with arrow: false it still happens:

Screen Shot 2019-11-05 at 12 51 32 AM
vesper8 commented 4 years ago

I guess the arrow spacing issue is because of the way the arrow positioning is arranged in the https://shepherdjs.dev/ theme:

  .shepherd-element.shepherd-element-attached-middle.shepherd-element-attached-right .shepherd-arrow,
  .shepherd-element.shepherd-pinned-left .shepherd-arrow {
    right: -35px;
  }

  .shepherd-element.shepherd-element-attached-middle.shepherd-element-attached-left .shepherd-arrow,
  .shepherd-element.shepherd-pinned-right .shepherd-arrow {
    left: -35px;
  }

As for the layout breakage, adding

.shepherd-target-marker {
  display: none;
}

Seems to fix that problem without having any visible adverse effects

I just need to figure out why the highlighting stopped working now and I'll be ready to fully upgrade to 6.0 : )

vesper8 commented 4 years ago

Figured out the highlighting... it had to do with how my custom overlay used to depend on the body having the shepherd-active class which it no longer has

Then this class does the job of the highlighting:

.shepherd-target.shepherd-enabled {
  box-shadow: 0px 0px 0px 10000px rgba(26, 163, 221, 0.4), inset 0px 0px 2px rgba(26, 163, 221, 0.4);
  position: relative;
  z-index: 10;
}

I have to say I'm a bit confused why it's necessary to add custom css to get the highlighting to work.. I must be doing something wrong

vesper8 commented 4 years ago

Last question if you please... I switched back to using the default arrows.. and I find that the shepherd element is much too close to the elements it's being tethered too.. on 5.0 there was more spacing and now it's almost no top. Is there a simple way to tell shepherd to allow for a little more spacing overall.. whether it's tethered via top, bottom, left or right, to overall have it be like 20px further away from the element it's attached to?

Right now I'm accomplishing this with:

  .shepherd-element.shepherd-element-attached-right {
    left: -30px !important;
  }  

  .shepherd-element.shepherd-element-attached-left {
    left: 30px !important;
  }  
RobbieTheWagner commented 4 years ago

There are a lot of posts here. Let's try to handle one thing at a time.

We could have an option to close the tour, if you click the modal overlay. It sounds like that is the only reason you have a custom one? If your custom one relied on the old modal overlay, it is not going to work right anymore. We switched from a mask to using a path for the opening.

Perhaps of less importance but I'd also like to note that previously the arrow aligned nicely.. whereas now the arrow is on top of the element it's supposed to be pointing too. Ideally the popup should be a little more spaced away from the item so that the arrow points TO the tether and isn't on top of it

You can infinitely customize offsets. You'll want to check out the Tether options and pass whatever offsets you desire for positioning.

vesper8 commented 4 years ago

Yea sorry for all the posts, as you'll see I was able to solve nearly all my issues. It would certainly be nice to have an option to close/cancel the tour when you click on the overlay, this has been asked before back in 2018 https://github.com/shipshapecode/shepherd/issues/141 and this is where I got my solution from. I've now been able to adapt it to 6.0 and it works as well as it did before.

I don't want to derail you from reading the above posts, I do feel I make some good points and might have pointed out some real issues. Regarding the shepherd-target-marker breaking the nav layout, and the highlight requiring a custom class (perhaps only in my case because of my custom overlay).

But the thing is I just stumbled onto another issue.. I'm being affected by https://github.com/shipshapecode/shepherd/issues/634 as well. My nav is near the top, but the shepherd popups always position themselves to the left or right of the nav items, so I don't understand why the arrow isn't appearing.. if I create a bit of padding at the top of the page it works fine. Really hope there's a workaround for these

RobbieTheWagner commented 4 years ago

@vesper8 what was asked in that old issue was a way to click outside the element and close the tour. That is different from clicking the overlay to close. We can definitely add a exitOnOverlayClick like introjs has.

As for the arrows issue, arrows currently only appear for steps where the arrow would go in the center. We certainly do need to work on some more robust arrow functionality.

I would highly recommend creating separate issues for bugs or feature requests, so we can correctly triage everything.

vesper8 commented 4 years ago

where is the underlying code that decides whether an arrow should appear or not (if it fits in the center as you said) ? I can't find it, I would like to take a shot at fixing this and making arrows work when they don't align perfectly.

RobbieTheWagner commented 4 years ago

@vesper8 the current arrow styles are here https://github.com/shipshapecode/shepherd/blob/master/src/js/components/shepherd-element.svelte#L114-L161

vesper8 commented 4 years ago

@rwwagner90 right.. I saw that.. but what I'm confused about is.. what causes the arrow not to work if it cannot fit perfectly in the center of the popup? Is this a css-only problem or is there some condition elsewhere that causes it not to appear in those cases?

RobbieTheWagner commented 4 years ago

@vesper8 if you check out those styles, they are based around tether attachment. Styles would have to be made for other attachments.

stale[bot] commented 4 years ago

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.