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

Feature request: stretched link #26184

Closed
wants to merge 3 commits into from

Conversation

MartijnCuppens
Copy link
Member

@MartijnCuppens MartijnCuppens commented Apr 2, 2018

Fixes #18294

Stretched links can make whole cards or other elements clickable. This PR is based on #25087, but can be used in multiple situations, not only cards.

Demo: https://deploy-preview-26184--twbs-bootstrap4.netlify.com/docs/4.1/utilities/stretched-link/

@XhmikosR

This comment has been minimized.

@XhmikosR

This comment has been minimized.

@XhmikosR XhmikosR force-pushed the stretched-link branch 2 times, most recently from 108474c to bb7b69b Compare June 25, 2018 18:15
@MartijnCuppens MartijnCuppens mentioned this pull request Jul 19, 2018
@XhmikosR XhmikosR requested a review from a team as a code owner November 4, 2018 11:39
@florianlacreuse
Copy link
Contributor

@MartijnCuppens Just to let you know that we can't select text in element containing a stretched link. Is this a known restriction?

@MartijnCuppens
Copy link
Member Author

@florianlacreuse, that's a known restricting. You can't select text in a link either (eg. see https://codepen.io/MartijnCuppens/pen/GwbWjp)

@mdo
Copy link
Member

mdo commented Dec 14, 2018

I'd like to bring this back for v4.2. Looking at it though, I wonder if we should do more to discourage multiple links and modifying their z-index. For inline anchors, including the example you've included here, if I hover over the space between each line of text, the stretched link will be clicked. This feels like it'd backfire for users who aren't watching the hover styles close enough.

Copy link
Member

@mdo mdo left a comment

Choose a reason for hiding this comment

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

Some copy suggestions here to be more specific.

site/docs/4.1/utilities/stretched-link.md Outdated Show resolved Hide resolved
site/docs/4.1/utilities/stretched-link.md Outdated Show resolved Hide resolved
site/docs/4.1/utilities/stretched-link.md Outdated Show resolved Hide resolved
site/docs/4.1/utilities/stretched-link.md Outdated Show resolved Hide resolved
site/docs/4.1/utilities/stretched-link.md Outdated Show resolved Hide resolved
site/docs/4.1/utilities/stretched-link.md Outdated Show resolved Hide resolved
@MartijnCuppens
Copy link
Member Author

MartijnCuppens commented Dec 16, 2018

Thanks for the copy changes, @mdo. Feel free to add an appropriate warning, I don't know if we should keep the example with the multiple links, because people might just copy paste this bad example?

@MartijnCuppens
Copy link
Member Author

The copy changes are looking good, thanks @mdo!

@mdo
Copy link
Member

mdo commented Dec 23, 2018

Build is failing because this is stuck in the 4.1 docs folder. I'll pull the branch down and tackle it.

@mdo mdo mentioned this pull request Dec 23, 2018
@XhmikosR
Copy link
Member

This was merged directly apparently. I don't agree with that solution since GH allows us to push to forks and pushing directly does NOT run the tests. Let alone you didn't squash.

@XhmikosR XhmikosR closed this Dec 23, 2018
@XhmikosR
Copy link
Member

git rebase -i is so easy, I don't get why you don't use it.

@MartijnCuppens
Copy link
Member Author

I agree, the v4-dev branch now contains some merge commits (like this one de7a382) which will make it unclear what the code is needed for.

@XhmikosR
Copy link
Member

XhmikosR commented Dec 23, 2018

@mdo: please disable merge commits for the whole repo. I'd also suggest that you disable direct pushes for admins too, no offense but you know :P

Joking aside, I don't know, you can leave the merge option although we don't really use it. I'm more concerned about the direct pushes.

@IAmHopp
Copy link

IAmHopp commented Dec 24, 2018

@mdo I know this is closed, but I think you meant "its containing block" instead of "it's containing block" on line eight of stretched-link.md.

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.

5 participants