From cb1cacebb59e2b03d37dcd8d34420d43cbf67db9 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Wed, 7 Jun 2017 16:45:49 +0100 Subject: [PATCH 01/14] set text layer to display:none for faster scroll --- .../web/public/stylesheets/app/editor/pdf.less | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/services/web/public/stylesheets/app/editor/pdf.less b/services/web/public/stylesheets/app/editor/pdf.less index 12c86a6190..3b745b6f2b 100644 --- a/services/web/public/stylesheets/app/editor/pdf.less +++ b/services/web/public/stylesheets/app/editor/pdf.less @@ -37,6 +37,20 @@ margin: 10px auto; padding: 0 10px; box-sizing: content-box; + // don't display the text layer by default, + // it slows down scrolling + div.plv-text-layer { + display: none; + } + // display it if we are hovering over the page + // so it's selectable + // FIXME: need to work out a solution to + // select text across multiple pages + &:hover { + div.plv-text-layer { + display: block; + } + } } } .progress-thin { From f0a940df356df699ed4fbe25180216bf147b79a8 Mon Sep 17 00:00:00 2001 From: Paulo Reis Date: Tue, 13 Jun 2017 10:22:47 +0100 Subject: [PATCH 02/14] Make the PDF text layer non-renderable by default. --- services/web/public/stylesheets/app/editor/pdf.less | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/services/web/public/stylesheets/app/editor/pdf.less b/services/web/public/stylesheets/app/editor/pdf.less index 3b745b6f2b..1976ab79c1 100644 --- a/services/web/public/stylesheets/app/editor/pdf.less +++ b/services/web/public/stylesheets/app/editor/pdf.less @@ -352,3 +352,13 @@ .files-dropdown { display: inline-block; } + +.plv-text-layer { + display: none; + & > div { + color: red; + } + &.plv-text-layer-visible { + display: block; + } +} From 1b07dda1b944cfc4f9525599a79581ef0f54b686 Mon Sep 17 00:00:00 2001 From: Paulo Reis Date: Tue, 13 Jun 2017 11:04:26 +0100 Subject: [PATCH 03/14] Use less specific (i.e. faster) selectors. --- .../web/public/stylesheets/app/editor/pdf.less | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/services/web/public/stylesheets/app/editor/pdf.less b/services/web/public/stylesheets/app/editor/pdf.less index 1976ab79c1..f185861b15 100644 --- a/services/web/public/stylesheets/app/editor/pdf.less +++ b/services/web/public/stylesheets/app/editor/pdf.less @@ -37,20 +37,6 @@ margin: 10px auto; padding: 0 10px; box-sizing: content-box; - // don't display the text layer by default, - // it slows down scrolling - div.plv-text-layer { - display: none; - } - // display it if we are hovering over the page - // so it's selectable - // FIXME: need to work out a solution to - // select text across multiple pages - &:hover { - div.plv-text-layer { - display: block; - } - } } } .progress-thin { From 786659dc1b0e9e028c4ab12f863134ee206b8cbb Mon Sep 17 00:00:00 2001 From: Paulo Reis Date: Tue, 13 Jun 2017 11:06:21 +0100 Subject: [PATCH 04/14] Render text layer on hovered pages (and adjacent ones). --- .../coffee/ide/pdfng/directives/pdfViewer.coffee | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee b/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee index ff4d5a4101..a8e1c6077b 100644 --- a/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee +++ b/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee @@ -405,6 +405,16 @@ define [ scope.adjustingScroll = true element.scrollTop(currentScrollTop + delta) + element.on 'mouseover', '.pdf-page-container', (e) -> + pdfPageEl = $(e.currentTarget) + + pdfPageEl.find('.plv-text-layer').addClass('plv-text-layer-visible') + pdfPageEl.prev().find('.plv-text-layer').addClass('plv-text-layer-visible') + pdfPageEl.next().find('.plv-text-layer').addClass('plv-text-layer-visible') + + element.on 'mouseout', '.pdf-page-container', (e) -> + $('.plv-text-layer-visible').removeClass('plv-text-layer-visible') + element.on 'scroll', () -> #console.log 'scroll event', element.scrollTop(), 'adjusting?', scope.adjustingScroll #scope.scrollPosition = element.scrollTop() From 4985f7ca1c690091915b186318d13c4c464c1e1a Mon Sep 17 00:00:00 2001 From: Paulo Reis Date: Tue, 13 Jun 2017 16:51:30 +0100 Subject: [PATCH 05/14] Different approach: render all text layers while selecting (WIP). --- .../ide/pdfng/directives/pdfViewer.coffee | 36 +++++++++++++++---- .../public/stylesheets/app/editor/pdf.less | 11 ++++-- 2 files changed, 37 insertions(+), 10 deletions(-) diff --git a/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee b/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee index a8e1c6077b..0dd9e58832 100644 --- a/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee +++ b/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee @@ -405,15 +405,37 @@ define [ scope.adjustingScroll = true element.scrollTop(currentScrollTop + delta) - element.on 'mouseover', '.pdf-page-container', (e) -> - pdfPageEl = $(e.currentTarget) + element.on 'mousedown', (e) -> + if !_hasSelection() + console.log 'adding class' + element.addClass 'pdfjs-viewer-show-text' + $(document.body).one 'mouseup', _handleSelectionMouseUp - pdfPageEl.find('.plv-text-layer').addClass('plv-text-layer-visible') - pdfPageEl.prev().find('.plv-text-layer').addClass('plv-text-layer-visible') - pdfPageEl.next().find('.plv-text-layer').addClass('plv-text-layer-visible') + _handleSelectionMouseUp = () -> + window.setTimeout _removeClassIfNoSelection, 10 - element.on 'mouseout', '.pdf-page-container', (e) -> - $('.plv-text-layer-visible').removeClass('plv-text-layer-visible') + _removeClassIfNoSelection = () -> + if _hasSelection() + console.log 'has selection, keeping class' + $(document.body).one 'mouseup', _handleSelectionMouseUp + else + console.log 'no selection, removing class' + element.removeClass 'pdfjs-viewer-show-text' + + _hasSelection = () -> + selection = window.getSelection() + if _isSelectionWithinPDF(selection) and selection.toString() != '' + console.log 'selection within and not empty' + selection.toString() + return true + else + console.log 'selection outside or empty' + return false + + _isSelectionWithinPDF = (selection) -> + if selection.rangeCount == 0 + return false + selectionAncestorNode = selection.getRangeAt(0).commonAncestorContainer + return element.find(selectionAncestorNode).length > 0 or element.is(selectionAncestorNode) element.on 'scroll', () -> #console.log 'scroll event', element.scrollTop(), 'adjusting?', scope.adjustingScroll diff --git a/services/web/public/stylesheets/app/editor/pdf.less b/services/web/public/stylesheets/app/editor/pdf.less index f185861b15..728e11d6a8 100644 --- a/services/web/public/stylesheets/app/editor/pdf.less +++ b/services/web/public/stylesheets/app/editor/pdf.less @@ -23,6 +23,7 @@ .full-size; background-color: @gray-lighter; overflow: scroll; + user-select: none; canvas, div.pdf-canvas { background: white; box-shadow: black 0px 0px 10px; @@ -341,10 +342,14 @@ .plv-text-layer { display: none; + user-select: text; + + .pdf-page-container:hover &, + .pdfjs-viewer-show-text & { + display: block; + } + & > div { color: red; } - &.plv-text-layer-visible { - display: block; - } } From fbaa918927ae5b398670e2a38564fa0e1c2a7508 Mon Sep 17 00:00:00 2001 From: Paulo Reis Date: Thu, 15 Jun 2017 11:34:24 +0100 Subject: [PATCH 06/14] Do not show the text layer when the user clicks outside pages. Remove some debug statements. --- .../ide/pdfng/directives/pdfViewer.coffee | 19 +++++++++---------- .../public/stylesheets/app/editor/pdf.less | 2 +- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee b/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee index 0dd9e58832..0543d64459 100644 --- a/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee +++ b/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee @@ -406,8 +406,14 @@ define [ element.scrollTop(currentScrollTop + delta) element.on 'mousedown', (e) -> - if !_hasSelection() - console.log 'adding class' + # We're checking that the event target isn't the directive root element + # to make sure that the click was within a PDF page - no point in showing + # the text layer when the click is outside. + # If the user clicks a PDF page, the mousedown target will be the canvas + # element (or the text layer one). Alternatively, if the event target is + # the root element, we can assume that the user has clicked either the + # grey background area or the scrollbars. + if e.target != element[0] and !_hasSelection() element.addClass 'pdfjs-viewer-show-text' $(document.body).one 'mouseup', _handleSelectionMouseUp @@ -416,20 +422,13 @@ define [ _removeClassIfNoSelection = () -> if _hasSelection() - console.log 'has selection, keeping class' $(document.body).one 'mouseup', _handleSelectionMouseUp else - console.log 'no selection, removing class' element.removeClass 'pdfjs-viewer-show-text' _hasSelection = () -> selection = window.getSelection() - if _isSelectionWithinPDF(selection) and selection.toString() != '' - console.log 'selection within and not empty' + selection.toString() - return true - else - console.log 'selection outside or empty' - return false + return _isSelectionWithinPDF(selection) and selection.toString() != '' _isSelectionWithinPDF = (selection) -> if selection.rangeCount == 0 diff --git a/services/web/public/stylesheets/app/editor/pdf.less b/services/web/public/stylesheets/app/editor/pdf.less index 728e11d6a8..284dfd81a1 100644 --- a/services/web/public/stylesheets/app/editor/pdf.less +++ b/services/web/public/stylesheets/app/editor/pdf.less @@ -23,7 +23,6 @@ .full-size; background-color: @gray-lighter; overflow: scroll; - user-select: none; canvas, div.pdf-canvas { background: white; box-shadow: black 0px 0px 10px; @@ -38,6 +37,7 @@ margin: 10px auto; padding: 0 10px; box-sizing: content-box; + user-select: none; } } .progress-thin { From 76dcde4dae69806add7b80b34e1e945cd0021d57 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 15 Jun 2017 13:42:19 +0100 Subject: [PATCH 07/14] hide the text layer on reload --- .../web/public/coffee/ide/pdfng/directives/pdfViewer.coffee | 2 ++ 1 file changed, 2 insertions(+) diff --git a/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee b/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee index 0543d64459..dd6e949f90 100644 --- a/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee +++ b/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee @@ -323,6 +323,8 @@ define [ clearTimeout spinnerTimer else spinner.remove(element) + # stop displaying the text layer + element.removeClass 'pdfjs-viewer-show-text' ctrl.redraw(origposition) $timeout renderVisiblePages scope.loadSuccess = true From b04288ed449a0759eca8f4570d4160a75ec72726 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 15 Jun 2017 14:11:08 +0100 Subject: [PATCH 08/14] avoid getting duplicate handlers for mouseup --- .../ide/pdfng/directives/pdfViewer.coffee | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee b/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee index dd6e949f90..ea521a74d1 100644 --- a/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee +++ b/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee @@ -417,16 +417,28 @@ define [ # grey background area or the scrollbars. if e.target != element[0] and !_hasSelection() element.addClass 'pdfjs-viewer-show-text' - $(document.body).one 'mouseup', _handleSelectionMouseUp + _setMouseUpHandler() + + mouseUpHandler = null # keep track of the handler to avoid adding multiple times + + _setMouseUpHandler = () -> + if not mouseUpHandler? + mouseUpHandler = $(document.body).one 'mouseup', _handleSelectionMouseUp _handleSelectionMouseUp = () -> - window.setTimeout _removeClassIfNoSelection, 10 + mouseUpHandler = null # reset handler, has now fired + window.setTimeout () -> + removedClass = _removeClassIfNoSelection() + # if we still have a selection we need to keep the handler going + if not removedClass then _setMouseUpHandler() + , 10 _removeClassIfNoSelection = () -> if _hasSelection() - $(document.body).one 'mouseup', _handleSelectionMouseUp + return false # didn't remove the text layer else element.removeClass 'pdfjs-viewer-show-text' + return true _hasSelection = () -> selection = window.getSelection() From b9804823f310ba8c83be35ffeffe393eb3ecd238 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 15 Jun 2017 14:13:15 +0100 Subject: [PATCH 09/14] handle off-screen selection when toggling the logs button the selection is off-screen and selection.toString() is empty even when there is a selected range. Can check for selection.type being "Range" instead. --- .../web/public/coffee/ide/pdfng/directives/pdfViewer.coffee | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee b/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee index ea521a74d1..ae9421cef3 100644 --- a/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee +++ b/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee @@ -442,7 +442,10 @@ define [ _hasSelection = () -> selection = window.getSelection() - return _isSelectionWithinPDF(selection) and selection.toString() != '' + # check the selection type in preference to using + # selection.toString() as the latter is "" when the + # selection is hidden (e.g. while viewing logs) + return _isSelectionWithinPDF(selection) and selection.type is 'Range' _isSelectionWithinPDF = (selection) -> if selection.rangeCount == 0 From 503822deb087a6e109292737cb906d8987f8102e Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 15 Jun 2017 14:32:00 +0100 Subject: [PATCH 10/14] avoid possible exception if selection is undefined defensive programming only --- .../web/public/coffee/ide/pdfng/directives/pdfViewer.coffee | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee b/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee index ae9421cef3..fe7ea3d427 100644 --- a/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee +++ b/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee @@ -441,11 +441,11 @@ define [ return true _hasSelection = () -> - selection = window.getSelection() + selection = window.getSelection?() # check the selection type in preference to using # selection.toString() as the latter is "" when the # selection is hidden (e.g. while viewing logs) - return _isSelectionWithinPDF(selection) and selection.type is 'Range' + return selection? and _isSelectionWithinPDF(selection) and selection.type is 'Range' _isSelectionWithinPDF = (selection) -> if selection.rangeCount == 0 From b9797dbc42f00c79304b1152f400d6e8f19b30a6 Mon Sep 17 00:00:00 2001 From: Paulo Reis Date: Thu, 15 Jun 2017 16:14:06 +0100 Subject: [PATCH 11/14] Make sure clicks on labels dont stop the event propagation to buttons. --- services/web/public/stylesheets/app/editor/toolbar.less | 1 + 1 file changed, 1 insertion(+) diff --git a/services/web/public/stylesheets/app/editor/toolbar.less b/services/web/public/stylesheets/app/editor/toolbar.less index 5f84f64682..1bb5f62bdc 100644 --- a/services/web/public/stylesheets/app/editor/toolbar.less +++ b/services/web/public/stylesheets/app/editor/toolbar.less @@ -10,6 +10,7 @@ right: 0; padding: .15em .6em .2em; font-size: 60%; + pointer-events: none; // Labels were capturing button/anchor clicks. } } From fcf0a969330daa1649ba7edaca74f89f56c47ac2 Mon Sep 17 00:00:00 2001 From: Paulo Reis Date: Thu, 15 Jun 2017 16:16:30 +0100 Subject: [PATCH 12/14] Use selection.isCollapsed to check for empty selections. --- .../public/coffee/ide/pdfng/directives/pdfViewer.coffee | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee b/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee index fe7ea3d427..241cee1c33 100644 --- a/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee +++ b/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee @@ -442,10 +442,10 @@ define [ _hasSelection = () -> selection = window.getSelection?() - # check the selection type in preference to using - # selection.toString() as the latter is "" when the - # selection is hidden (e.g. while viewing logs) - return selection? and _isSelectionWithinPDF(selection) and selection.type is 'Range' + # check the selection collapsed state in preference to + # using selection.toString() as the latter is "" when + # the selection is hidden (e.g. while viewing logs) + return selection? and _isSelectionWithinPDF(selection) and !selection.isCollapsed _isSelectionWithinPDF = (selection) -> if selection.rangeCount == 0 From af2cbf5a513e9e4a41af658d802898f5794a8fc9 Mon Sep 17 00:00:00 2001 From: Paulo Reis Date: Thu, 15 Jun 2017 16:17:21 +0100 Subject: [PATCH 13/14] Force return true on a jquery handler; returning falsy values may have unexpected results. --- services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee | 1 + 1 file changed, 1 insertion(+) diff --git a/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee b/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee index 241cee1c33..e6401b11ac 100644 --- a/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee +++ b/services/web/public/coffee/ide/pdfng/directives/pdfViewer.coffee @@ -432,6 +432,7 @@ define [ # if we still have a selection we need to keep the handler going if not removedClass then _setMouseUpHandler() , 10 + return true _removeClassIfNoSelection = () -> if _hasSelection() From 412c8234833c9c3c27c1d6f774ae42e851603687 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Mon, 19 Jun 2017 14:58:22 +0100 Subject: [PATCH 14/14] remove the red highlighting of the text layer --- services/web/public/stylesheets/app/editor/pdf.less | 4 ---- 1 file changed, 4 deletions(-) diff --git a/services/web/public/stylesheets/app/editor/pdf.less b/services/web/public/stylesheets/app/editor/pdf.less index 284dfd81a1..0c98050242 100644 --- a/services/web/public/stylesheets/app/editor/pdf.less +++ b/services/web/public/stylesheets/app/editor/pdf.less @@ -348,8 +348,4 @@ .pdfjs-viewer-show-text & { display: block; } - - & > div { - color: red; - } }