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

local branch of Print Dropdown for Docs (Outlines/Color) #5927

Open
wants to merge 25 commits into
base: master
Choose a base branch
from
Open

local branch of Print Dropdown for Docs (Outlines/Color) #5927

wants to merge 25 commits into from

Conversation

pelikhan
Copy link
Member

See #5515

Alejandro Toledo and others added 18 commits April 24, 2019 18:33
@pelikhan pelikhan requested review from abchatra and darzu September 19, 2019 21:46
jwunderl
jwunderl requested changes Sep 19, 2019
View reviewed changes
Copy link
Member

@jwunderl jwunderl left a comment

Choose a reason for hiding this comment

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

this would cause a minor keyboard accessibility regression; should probably fix that before deploying

docfiles/docs.js Outdated Show resolved Hide resolved
docfiles/docs.js Outdated Show resolved Hide resolved
docfiles/docs.js Outdated
window.print();
})
$('#printbtn').dropdown({
action: function(text, value) {
Copy link
Member

@jwunderl jwunderl Sep 19, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

this doesn't seem to activate on space bar in chrome, only enter key

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this still the case?

Copy link
Member

Choose a reason for hiding this comment

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

Yup, still won't activate on space

Copy link
Contributor

Choose a reason for hiding this comment

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

I took a stab at fixing this but couldn't find a working solution. It's even broken on semantic UI's demo page: https://semantic-ui.com/modules/dropdown.html#/examples
I vote we punt this to another work item

docfiles/printbtn.html Outdated Show resolved Hide resolved
jwunderl
jwunderl approved these changes Sep 30, 2019
View reviewed changes
Copy link
Member

@jwunderl jwunderl left a comment

Choose a reason for hiding this comment

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

Needs fixing still but I'll do it later

@darzu darzu mentioned this pull request Sep 30, 2019
Closed
@darzu
Copy link
Contributor

darzu commented Sep 30, 2019

Things look broken after the new docs update:
image

and selecting "color" didn't print with color. Can't merge this as-is.

@darzu darzu self-requested a review September 30, 2019 22:07
darzu
darzu suggested changes Sep 30, 2019
View reviewed changes
Copy link
Contributor

@darzu darzu left a comment

Choose a reason for hiding this comment

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

see comments

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@darzu darzu darzu requested changes

@jwunderl jwunderl jwunderl approved these changes

@riknoll riknoll Awaiting requested review from riknoll

@abchatra abchatra Awaiting requested review from abchatra

Assignees

@darzu darzu

Labels
documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pelikhan @darzu @jwunderl