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

Clicking outside of about card dismisses card #547

Merged
merged 2 commits into from Jun 11, 2019

Conversation

@bmitchinson
Copy link
Contributor

commented May 28, 2019

This is currently how I have my site configured, and seemed to be the preferred / instinctive way to interact with the card. On desktop, most people clicked outside of the card hoping it would dismiss, and then spent time searching for the X button, or attempting to go backwards in the browser, navigating away from the page entirely,

Implementation live for example on https://www.benmitchinson.com


This change is Reviewable

@bmitchinson

This comment has been minimized.

Copy link
Contributor Author

commented Jun 9, 2019

I've since removed this from my site, there's a small mistake I can't seem to correct.

Because the AboutCard is resting on top of the About element, somehow whenever I click on the card itself, or any links inside, the card closes (As if I'm clicking on the background). Is there a way to play with Z-Index to get around this?

To clarify, I'm trying to dismiss the about card by clicking on the background. (Without breaking link functionality within my bio)

Video of the case attached https://res.cloudinary.com/dheqbiqti/video/upload/v1560096125/Random%20Sharing/AboutDismiss.mp4

@LouisBarranqueiro
Copy link
Owner

left a comment

👋 @bmitchinson , I agree, it really improves UX 👍

Reviewed 2 of 2 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @bmitchinson)


source/_css/layouts/_about.scss, line 26 at r1 (raw file):

        border-radius: 3px;
        padding:       30px 0;
        cursor:        default;

it needs to be initial and not default because we also have texts and links in this card so the cursor must be different in these situations


source/_js/about.js, line 40 at r1 (raw file):

        e.preventDefault();
        self.playBack();
      });

we also need to add:

self.$aboutCard.click(function(event) {
    event.stopPropagation();
});

to not close the about page when users click on the card.

@bmitchinson

This comment has been minimized.

Copy link
Contributor Author

commented Jun 10, 2019

@LouisBarranqueiro Just added that fix, thanks so much for a clean solution 👍

@LouisBarranqueiro
Copy link
Owner

left a comment

code :lgtm:

Reviewed 2 of 2 files at r2.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@LouisBarranqueiro

This comment has been minimized.

Copy link
Owner

commented Jun 11, 2019

tests :lgtm:

@LouisBarranqueiro LouisBarranqueiro merged commit 52c0775 into LouisBarranqueiro:dev Jun 11, 2019

1 check passed

code-review/reviewable 2 files reviewed
Details

LouisBarranqueiro added a commit that referenced this pull request Jul 1, 2019

LouisBarranqueiro added a commit that referenced this pull request Jul 1, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.