Skip to content

Disable outline button when unavailable#3054

Merged
brendandahl merged 1 commit into
mozilla:masterfrom
saebekassebil:disabled
Apr 11, 2013
Merged

Disable outline button when unavailable#3054
brendandahl merged 1 commit into
mozilla:masterfrom
saebekassebil:disabled

Conversation

@saebekassebil
Copy link
Copy Markdown
Contributor

This is a little accessibility that bugs me, the outline button is clickable although there's no outline available. This PR disables the button (but still creates the DocumentOutlineView).

Thoughts on this? If you like this, maybe I should just prevent the creation of a DocumentOutlineView in cases where there's no outline?

Comment thread web/viewer.js Outdated
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

A shorter solution for this would be: document.getElementById('viewOutline').disabled = !outline;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Quite right.

@Snuffleupagus
Copy link
Copy Markdown
Collaborator

Personally I like this idea!
If this is a change that the developers agree should be done, I would suggest adding disabled to the HTML tag: https://github.com/mozilla/pdf.js/blob/master/web/viewer.html#L85. This would prevent the the button from appearing click-able before the outline has been loaded.

@waddlesplash
Copy link
Copy Markdown
Contributor

/botio-linux preview

@pdfjsbot
Copy link
Copy Markdown

pdfjsbot commented Apr 9, 2013

From: Bot.io (Linux)


Received

Command cmd_preview from @waddlesplash received. Current queue size: 0

Live output at: http://107.21.233.14:8877/55620789660af52/output.txt

@pdfjsbot
Copy link
Copy Markdown

pdfjsbot commented Apr 9, 2013

@waddlesplash
Copy link
Copy Markdown
Contributor

Very nice!
You can also remove the "No outline available" from all L10N files as it won't be needed anymore, and the text span and CSS rules for it.

@saebekassebil
Copy link
Copy Markdown
Contributor Author

@waddlesplash I've removed the no_outline locale property, per 6fbc8bc, .noOutline per 51abfe6 and finally assured that the outline view in the sidebar, cannot be visible when no outline is available per afb039a

@waddlesplash
Copy link
Copy Markdown
Contributor

/botio-linux preview

@pdfjsbot
Copy link
Copy Markdown

pdfjsbot commented Apr 9, 2013

From: Bot.io (Linux)


Received

Command cmd_preview from @waddlesplash received. Current queue size: 1

Live output at: http://107.21.233.14:8877/50f761e11ae9955/output.txt

@pdfjsbot
Copy link
Copy Markdown

pdfjsbot commented Apr 9, 2013

@brendandahl
Copy link
Copy Markdown
Contributor

Squash then good to go.

brendandahl added a commit that referenced this pull request Apr 11, 2013
Disable outline button when unavailable
@brendandahl brendandahl merged commit 9853079 into mozilla:master Apr 11, 2013
@xixixao
Copy link
Copy Markdown

xixixao commented May 26, 2013

The button is still sort-of clickable, at least that's the graphics for it. Actually, even the active button is clickable and that leads me to think that it has some action (like closing the sidebar), when in fact it doesn't.

What about removing both buttons altogether when only one options is available? Imo would provide better experience.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants