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.

Sign up for GitHub

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

Jump to bottom

Update modal docs #30325

Merged
merged 5 commits into from
Mar 23, 2020
Merged

Update modal docs #30325

merged 5 commits into from
Mar 23, 2020

Conversation

ysds
Copy link
Contributor

@ysds ysds commented Mar 2, 2020

  • Simplify modal snippets
  • Use id for data-target
  • Unify the id positions

Closes #30055

@ysds ysds requested review from a team and XhmikosR March 2, 2020 14:56
@XhmikosR XhmikosR requested a review from MartijnCuppens March 2, 2020 14:58
@ysds ysds changed the title Master ysds modal docs Update modal docs Mar 2, 2020
@MartijnCuppens
Copy link
Member

Imo, this PR removes a little too much. I think people like it to just copy-paste the whole modal.

@ysds
Copy link
Contributor Author

ysds commented Mar 4, 2020 •
edited
Loading

I do not think so. The first snippet in the "Scrolling long content" section makes no sense. Also, the current snippets of the size options don't work (broken) already because there are no content in the .modal-content. To make all snippets that can be copied successfully need too long line.

@XhmikosR
Copy link
Member

XhmikosR commented Mar 4, 2020

I'm in between personally; I see @ysds's and @MartijnCuppens's points.

/CC @mdo to the rescue :P

@XhmikosR XhmikosR requested a review from mdo March 4, 2020 13:46
@patrickhlauke
Copy link
Member

FWIW i don't mind the cuts here

mdo
mdo approved these changes Mar 20, 2020
View reviewed changes
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.

Cut it! :D

MartijnCuppens
MartijnCuppens approved these changes Mar 23, 2020
View reviewed changes
Copy link
Member

@MartijnCuppens MartijnCuppens left a comment

Choose a reason for hiding this comment

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

Fine by me, let's do this

@MartijnCuppens MartijnCuppens merged commit 1b7e13f into master Mar 23, 2020
@MartijnCuppens MartijnCuppens deleted the master-ysds-modal-docs branch March 23, 2020 17:15
XhmikosR pushed a commit that referenced this pull request Mar 25, 2020
@ysds @XhmikosR
Update modal docs (#30325)
f2b058d
XhmikosR pushed a commit that referenced this pull request Mar 25, 2020
@ysds @XhmikosR
Update modal docs (#30325)
f9ee54b
XhmikosR pushed a commit that referenced this pull request Mar 28, 2020
@ysds @XhmikosR
Update modal docs (#30325)
e616166
XhmikosR pushed a commit that referenced this pull request Mar 31, 2020
@ysds @XhmikosR
Update modal docs (#30325)
200a383
XhmikosR pushed a commit that referenced this pull request Mar 31, 2020
@ysds @XhmikosR
Update modal docs (#30325)
effe4e9
olsza pushed a commit to olsza/bootstrap that referenced this pull request Oct 3, 2020
@ysds @olsza
Update modal docs (twbs#30325)
7557b26
@nelsonni nelsonni mentioned this pull request Jan 5, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mdo mdo mdo approved these changes

@MartijnCuppens MartijnCuppens MartijnCuppens approved these changes

@XhmikosR XhmikosR Awaiting requested review from XhmikosR

Assignees
No one assigned
Labels
backport-to-v4 docs v4 v5
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Modal Documentation for Vertically Centered isn't Correct
5 participants
@ysds @MartijnCuppens @XhmikosR @patrickhlauke @mdo