Show direct links on stream page when available (FIRST!) #1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
finitud
changed the title
|
you forgot to say FIRST! 👆 |
finitud
changed the title
|
@lenazun: fixed that :-p |
|
|
||
| describe('documentTitle', function() { | ||
|
|
||
| it('returns the title linked if the document has title and uri', function() { |
There was a problem hiding this comment.
I think all the tests in here except the two "escapes HTML" tests will need equivalents for the new function - hopefully they should be somewhat simpler. Couple of other things to note when writing replacement tests:
- Use
assert.equal()orassert.<other predicate>rather thanassert(condition). The latter blows up horribly if the test fails, unlike in our Python tests - Consider making use of
unroll()when you have a set of tests that are the same except for the parameters to a function call.
There was a problem hiding this comment.
I've opted for not using unroll(), since I'm not sure whether it'd make tests clearer in this particular case, but open to reorganise with it.
There was a problem hiding this comment.
The revised tests look clear to me. If we had a lot more cases then using a data-driven test is a useful way to make things more concise.
|
After the refactoring I believe that |
Move relevant tests to annotation-metadata-test.js, and fix a bug in annotation-metadata.js.
finitud
changed the title
|
LGTM |
sheetaluk
pushed a commit
that referenced
this pull request
Aug 4, 2016
Default to the annotations tab when user switches between groups, clears a selection or reloads a page. The ANNOTATIONS_SYNCED event is emitted when annotations for a document have finished anchoring. Because annotations are not marked as orphans until they fail to anchor, select the annotations tab after they have finished anchoring. Test cases: ------------- 1. On page load localhost:3000 annotations tab is selected. clicking on new note switches to the notes tab with a new draft created. switching between tabs maintains tab selection, if notes or annotations. when tab selected is orphans, changing groups defaults to the annotations tab. 2. On page load localhost:3000/#annotations:<id> clicking on new page note should select the notes tab. When an annotation/note is selected, the tab selection carries between group changes. all tests listed in #1 above should apply. 3. On page load localhost:3000/#annotations:<orphan_id> clicking on “show all annotations and notes” selects the annotations tab and displays all annotations. all tests listed in #1 and #2 above should apply. switching between groups selects the annotations tab by default https://trello.com/c/1NVK2jwv/400-add-a-tab-for-unanchored-annotations https://trello.com/c/OLdLTlLT/342-separate-annotations-and-notes Hide orphans tab behind orphans_tab flag. Fix bug where notes tab isn't selected when user attempts to create a new page note.
sheetaluk
pushed a commit
that referenced
this pull request
Aug 4, 2016
Default to the annotations tab when user switches between groups, clears a selection or reloads a page. The ANNOTATIONS_SYNCED event is emitted when annotations for a document have finished anchoring. Because annotations are not marked as orphans until they fail to anchor, select the annotations tab after they have finished anchoring. Test cases: ------------- 1. On page load localhost:3000 annotations tab is selected. clicking on new note switches to the notes tab with a new draft created. switching between tabs maintains tab selection, if notes or annotations. when tab selected is orphans, changing groups defaults to the annotations tab. 2. On page load localhost:3000/#annotations:<id> clicking on new page note should select the notes tab. When an annotation/note is selected, the tab selection carries between group changes. all tests listed in #1 above should apply. 3. On page load localhost:3000/#annotations:<orphan_id> clicking on “show all annotations and notes” selects the annotations tab and displays all annotations. all tests listed in #1 and #2 above should apply. switching between groups selects the annotations tab by default https://trello.com/c/1NVK2jwv/400-add-a-tab-for-unanchored-annotations https://trello.com/c/OLdLTlLT/342-separate-annotations-and-notes Hide orphans tab behind orphans_tab flag. Fix bug where notes tab isn't selected when user attempts to create a new page note.
robertknight
pushed a commit
that referenced
this pull request
Aug 4, 2016
Default to the annotations tab when user switches between groups, clears a selection or reloads a page. The ANNOTATIONS_SYNCED event is emitted when annotations for a document have finished anchoring. Because annotations are not marked as orphans until they fail to anchor, select the annotations tab after they have finished anchoring. Test cases: ------------- 1. On page load localhost:3000 annotations tab is selected. clicking on new note switches to the notes tab with a new draft created. switching between tabs maintains tab selection, if notes or annotations. when tab selected is orphans, changing groups defaults to the annotations tab. 2. On page load localhost:3000/#annotations:<id> clicking on new page note should select the notes tab. When an annotation/note is selected, the tab selection carries between group changes. all tests listed in #1 above should apply. 3. On page load localhost:3000/#annotations:<orphan_id> clicking on “show all annotations and notes” selects the annotations tab and displays all annotations. all tests listed in #1 and #2 above should apply. switching between groups selects the annotations tab by default https://trello.com/c/1NVK2jwv/400-add-a-tab-for-unanchored-annotations https://trello.com/c/OLdLTlLT/342-separate-annotations-and-notes Hide orphans tab behind orphans_tab flag. Fix bug where notes tab isn't selected when user attempts to create a new page note.
sheetaluk
pushed a commit
that referenced
this pull request
Aug 4, 2016
Default to the annotations tab when user switches between groups, clears a selection or reloads a page. The ANNOTATIONS_SYNCED event is emitted when annotations for a document have finished anchoring. Because annotations are not marked as orphans until they fail to anchor, select the annotations tab after they have finished anchoring. Test cases: ------------- 1. On page load localhost:3000 annotations tab is selected. clicking on new note switches to the notes tab with a new draft created. switching between tabs maintains tab selection, if notes or annotations. when tab selected is orphans, changing groups defaults to the annotations tab. 2. On page load localhost:3000/#annotations:<id> clicking on new page note should select the notes tab. When an annotation/note is selected, the tab selection carries between group changes. all tests listed in #1 above should apply. 3. On page load localhost:3000/#annotations:<orphan_id> clicking on “show all annotations and notes” selects the annotations tab and displays all annotations. all tests listed in #1 and #2 above should apply. switching between groups selects the annotations tab by default https://trello.com/c/1NVK2jwv/400-add-a-tab-for-unanchored-annotations https://trello.com/c/OLdLTlLT/342-separate-annotations-and-notes Hide orphans tab behind orphans_tab flag. Fix bug where notes tab isn't selected when user attempts to create a new page note.
robertknight
pushed a commit
that referenced
this pull request
Aug 4, 2016
Default to the annotations tab when user switches between groups, clears a selection or reloads a page. The ANNOTATIONS_SYNCED event is emitted when annotations for a document have finished anchoring. Because annotations are not marked as orphans until they fail to anchor, select the annotations tab after they have finished anchoring. Test cases: ------------- 1. On page load localhost:3000 annotations tab is selected. clicking on new note switches to the notes tab with a new draft created. switching between tabs maintains tab selection, if notes or annotations. when tab selected is orphans, changing groups defaults to the annotations tab. 2. On page load localhost:3000/#annotations:<id> clicking on new page note should select the notes tab. When an annotation/note is selected, the tab selection carries between group changes. all tests listed in #1 above should apply. 3. On page load localhost:3000/#annotations:<orphan_id> clicking on “show all annotations and notes” selects the annotations tab and displays all annotations. all tests listed in #1 and #2 above should apply. switching between groups selects the annotations tab by default https://trello.com/c/1NVK2jwv/400-add-a-tab-for-unanchored-annotations https://trello.com/c/OLdLTlLT/342-separate-annotations-and-notes Hide orphans tab behind orphans_tab flag. Fix bug where notes tab isn't selected when user attempts to create a new page note.
robertknight
pushed a commit
that referenced
this pull request
Feb 16, 2017
correcting CC0 terminology
robertknight
added a commit
that referenced
this pull request
Apr 17, 2020
This commit fixes an issue with the client's Jenkins pipeline that could cause code for a recent but not-the-latest commit to master to be deployed to QA, but with the version number corresponding to the latest commit. --- The client's Jenkins CI/CD build is split into three phases: 1. Checkout source, build, lint and test 2. Publish QA release 3. Publish prod release Each of these phases was executed in a separate `node` block. Each node block defines a task and Jenkins allocates a workspace directory for that task. Workspace directories are named after the project, branch and concurrent build number (eg. `/data/jenkins/workspace/client_master@2`). Each workspace directory is "locked" for use by one task at a time, but workspace directories are re-used when not locked. New workspace directories are allocated as needed when concurrent builds happen and all existing directories for a given (project, branch) are locked. If there was only one active build of the client's "master" branch, phases (2) and (3) would re-use the same workspace directory and everything would work as expected. If however there was a concurrent build, phase (2) could end up using a different workspace directory than phase (1): 1. Build A starts phase 1 and uses workspace dir #1 2. Build B starts phase 1 and uses workspace dir #2 3. Build A finishes phase 1, freeing workspace #1 4. Build A starts phase 2, using workspace #1 5. Build A completes phase 2, freeing workspace #1 6. Build B completes phase 1 and starts phase 2, using workspace #1 In step (6), build B runs phase 2 (the "Publish QA" step) using the workspace created by build A and publishes the code checked out by build A with the version number associated with build B. The fact that this kind of screw-up is even possible shows a deeper issue with our current CI setup. Nevertheless, this commit addresses the immediate issue by: 1. Putting phases (1) and (2) in the same `node` block so that they are guaranteed to use the same workspace directory. There was no benefit to making them separate tasks. 2. Adding a `checkout scm` command at the start of phase (3) so that it will definitely deploy the same code that was tested and deployed to QA in steps (1) and (2) [1] https://github.com/jenkinsci/pipeline-plugin/blob/master/TUTORIAL.md#understanding-syntax
esanzgar
added a commit
that referenced
this pull request
Jan 19, 2021
Currently, if the client's sidebar is opened it doesn't behave graciously when the browser window is resized. Sometimes the sidebar appears floating on the middle of the window. We discussed several alternative solutions. When the sidebar is opened and the window is resized, then: 1. close the sidebar and the user opens it manually 2. maintain the sidebar open but scale it properly 3. close the sidebar and open it after X milliseconds of the last resize event 4. close the sidebar if the width of the window is reduced, but maintain it open if the window's width is increased We implemented solution #1. However, this issue is still visible on the PDF-sidebar. It would be great if the both sidebars (PDF and non-PDF) have the same behaviour. In addition, I added a mechanism to register and unregister events, to avoid resource leaks.
esanzgar
added a commit
that referenced
this pull request
Jan 21, 2021
Currently, if the client's sidebar is opened it doesn't behave graciously when the browser window is resized. Sometimes the sidebar appears floating on the middle of the window. We discussed several alternative solutions: when the sidebar is opened and the window is resized, then... 1. close the sidebar and the user opens it manually 2. maintain the sidebar open, but scale it properly 3. close the sidebar and open it after X milliseconds of the last resize event 4. close the sidebar if the width of the window is reduced, but maintain it open if the window's width is increased We implemented solution #1. However, this issue is still visible on the PDF-sidebar. It would be great if the both sidebars (PDF and non-PDF) have the same behaviour. In addition, I added a mechanism to register and unregister events, to avoid resource leaks.
esanzgar
added a commit
that referenced
this pull request
Jan 21, 2021
Currently, if the client's sidebar is opened it doesn't behave graciously when the browser window is resized. Sometimes the sidebar appears floating on the middle of the window. We discussed several alternative solutions: when the sidebar is opened and the window is resized, then... 1. close the sidebar and the user opens it manually 2. maintain the sidebar open, but scale it properly 3. close the sidebar and open it after X milliseconds of the last resize event 4. close the sidebar if the width of the window is reduced, but maintain it open if the window's width is increased We implemented solution #1. However, this issue is still visible on the PDF-sidebar. It would be great if the both sidebars (PDF and non-PDF) have the same behaviour. In addition, I added a mechanism to register and unregister events, to avoid resource leaks.
esanzgar
added a commit
that referenced
this pull request
Jun 21, 2021
On this prototype, we start using `FrameConnector` on the `host` frame
and `PortFinder` on `SidebarView` and `NotebookView` components (on the
`sidebar` and `notebook` iframes, respectively)
`FrameConnector` creates `MessageChannel` for the communication between
two frames.
There are 4 types of frames:
- `host`: frame where the client is initially loaded
- `sidebar`: the main hypothesis app. It runs on an iframe and is
generally shadow DOMed. It is responsible for communicating with the
backend, fetching and saving the annotations.
- `notebook`: it is an another hypothesis app. It runs on an iframe and
is generally shadow DOMed.
- `guest/s`: iframes with annotatable content
This layout represents the current arrangement of frames:
```
`host` frame
|-> (generally, shadow DOMed) `sidebar` iframe
|-> (generally, shadow DOMed) `notebook` iframe
|-> [`guest` iframe/s]
|-> [`guest` iframe/s]
```
`FrameConnector` runs only on the `host` frame. The rest of the frames run the companion class, `PortFinder`. `FrameConnector` creates `MessageChannels` for
two frames to communicate with each other. It also listens to requests
for particular channel.port and dispatches the corresponding `MessagePort`.
```
FrameConnector | PortFinder
----------------------------------------------------|--------------------------------------------------------------
2. listens to `requests` of channel.port <--------------------- 1. `request` a channel.port using `host.postMessage`
|
V
3. sends `offers` of channel.port using `frame.postMessage` ---> 4. listens to `offers` of channel.port
```
It is essential that `FrameConnect` initialize the listeners (step #2)
before `PortFinder` sends the `requests` of channel.port (step #1).
Because the `host` frame creates the `sidebar` and `notebook` iframes,
it is guaranteed that `host` frame is ready to listen to messages
originating from the `sidebar` and `notebook` iframes.
However, the above assumption is not true for `host` and `guest` frames.
Because `guest` iframes load in parallel, we can not assume that the
code in the `host` frame is executed before the code in a `guest` frame.
Therefore, for the `guest` frames we implement a polling strategy
(sending a message every N milliseconds until a response is received).
Next step would be to extend the prototype to connect other frmes:
- `host` <-> `sidebar`
- `guest` <-> `sidebar`
esanzgar
added a commit
that referenced
this pull request
Jun 21, 2021
On this prototype, we start using `FrameConnector` on the `host` frame
and `PortFinder` on `SidebarView` and `NotebookView` components (on the
`sidebar` and `notebook` iframes, respectively)
`FrameConnector` creates `MessageChannel` for the communication between
two frames.
There are 4 types of frames:
- `host`: frame where the client is initially loaded
- `sidebar`: the main hypothesis app. It runs on an iframe and is
generally shadow DOMed. It is responsible for communicating with the
backend, fetching and saving the annotations.
- `notebook`: it is an another hypothesis app. It runs on an iframe and
is generally shadow DOMed.
- `guest/s`: iframes with annotatable content
This layout represents the current arrangement of frames:
```
`host` frame
|-> (generally, shadow DOMed) `sidebar` iframe
|-> (generally, shadow DOMed) `notebook` iframe
|-> [`guest` iframe/s]
|-> [`guest` iframe/s]
```
`FrameConnector` runs only on the `host` frame. The rest of the frames run the companion class, `PortFinder`. `FrameConnector` creates `MessageChannels` for
two frames to communicate with each other. It also listens to requests
for particular channel.port and dispatches the corresponding `MessagePort`.
```
FrameConnector | PortFinder
----------------------------------------------------|--------------------------------------------------------------
2. listens to `requests` of channel.port <--------------------- 1. `request` a channel.port using `host.postMessage`
|
V
3. sends `offers` of channel.port using `frame.postMessage` ---> 4. listens to `offers` of channel.port
```
It is essential that `FrameConnect` initialize the listeners (step #2)
before `PortFinder` sends the `requests` of channel.port (step #1).
Because the `host` frame creates the `sidebar` and `notebook` iframes,
it is guaranteed that `host` frame is ready to listen to messages
originating from the `sidebar` and `notebook` iframes.
However, the above assumption is not true for `host` and `guest` frames.
Because `guest` iframes load in parallel, we can not assume that the
code in the `host` frame is executed before the code in a `guest` frame.
Therefore, for the `guest` frames we implement a polling strategy
(sending a message every N milliseconds until a response is received).
Next step would be to extend the prototype to connect other frmes:
- `host` <-> `sidebar`
- `guest` <-> `sidebar`
esanzgar
added a commit
that referenced
this pull request
Jun 21, 2021
On this prototype, we start using `FrameConnector` on the `host` frame
and `PortFinder` on `SidebarView` and `NotebookView` components (on the
`sidebar` and `notebook` iframes, respectively)
`FrameConnector` creates `MessageChannel` for the communication between
two frames.
There are 4 types of frames:
- `host`: frame where the client is initially loaded
- `sidebar`: the main hypothesis app. It runs on an iframe and is
generally shadow DOMed. It is responsible for communicating with the
backend, fetching and saving the annotations.
- `notebook`: it is an another hypothesis app. It runs on an iframe and
is generally shadow DOMed.
- `guest/s`: iframes with annotatable content
This layout represents the current arrangement of frames:
```
`host` frame
|-> (generally, shadow DOMed) `sidebar` iframe
|-> (generally, shadow DOMed) `notebook` iframe
|-> [`guest` iframe/s]
|-> [`guest` iframe/s]
```
`FrameConnector` runs only on the `host` frame. The rest of the frames run the companion class, `PortFinder`. `FrameConnector` creates `MessageChannels` for
two frames to communicate with each other. It also listens to requests
for particular channel.port and dispatches the corresponding `MessagePort`.
```
FrameConnector | PortFinder
----------------------------------------------------|--------------------------------------------------------------
2. listens to `requests` of channel.port <--------------------- 1. `request` a channel.port using `host.postMessage`
|
V
3. sends `offers` of channel.port using `frame.postMessage` ---> 4. listens to `offers` of channel.port
```
It is essential that `FrameConnect` initialize the listeners (step #2)
before `PortFinder` sends the `requests` of channel.port (step #1).
Because the `host` frame creates the `sidebar` and `notebook` iframes,
it is guaranteed that `host` frame is ready to listen to messages
originating from the `sidebar` and `notebook` iframes.
However, the above assumption is not true for `host` and `guest` frames.
Because `guest` iframes load in parallel, we can not assume that the
code in the `host` frame is executed before the code in a `guest` frame.
Therefore, for the `guest` frames we implement a polling strategy
(sending a message every N milliseconds until a response is received).
Next step would be to extend the prototype to connect other frmes:
- `host` <-> `sidebar`
- `guest` <-> `sidebar`
esanzgar
added a commit
that referenced
this pull request
Jun 21, 2021
On this prototype, we start using `FrameConnector` on the `host` frame
and `PortFinder` on `SidebarView` and `NotebookView` components (on the
`sidebar` and `notebook` iframes, respectively)
`FrameConnector` creates `MessageChannel` for the communication between
two frames.
There are 4 types of frames:
- `host`: frame where the client is initially loaded
- `sidebar`: the main hypothesis app. It runs on an iframe and is
generally shadow DOMed. It is responsible for communicating with the
backend, fetching and saving the annotations.
- `notebook`: it is an another hypothesis app. It runs on an iframe and
is generally shadow DOMed.
- `guest/s`: iframes with annotatable content
This layout represents the current arrangement of frames:
```
`host` frame
|-> (generally, shadow DOMed) `sidebar` iframe
|-> (generally, shadow DOMed) `notebook` iframe
|-> [`guest` iframe/s]
|-> [`guest` iframe/s]
```
`FrameConnector` runs only on the `host` frame. The rest of the frames run the companion class, `PortFinder`. `FrameConnector` creates `MessageChannels` for
two frames to communicate with each other. It also listens to requests
for particular channel.port and dispatches the corresponding `MessagePort`.
```
FrameConnector | PortFinder
----------------------------------------------------|--------------------------------------------------------------
2. listens to `requests` of channel.port <--------------------- 1. `request` a channel.port using `host.postMessage`
|
V
3. sends `offers` of channel.port using `frame.postMessage` ---> 4. listens to `offers` of channel.port
```
It is essential that `FrameConnect` initialize the listeners (step #2)
before `PortFinder` sends the `requests` of channel.port (step #1).
Because the `host` frame creates the `sidebar` and `notebook` iframes,
it is guaranteed that `host` frame is ready to listen to messages
originating from the `sidebar` and `notebook` iframes.
However, the above assumption is not true for `host` and `guest` frames.
Because `guest` iframes load in parallel, we can not assume that the
code in the `host` frame is executed before the code in a `guest` frame.
Therefore, for the `guest` frames we implement a polling strategy
(sending a message every N milliseconds until a response is received).
Next step would be to extend the prototype to connect other frmes:
- `host` <-> `sidebar`
- `guest` <-> `sidebar`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently the links on the stream page point to the original annotated document. This pull request makes these links to be direct links to the annotation.
https://trello.com/c/iipUgujB/373-direct-link-to-annotation-from-stream-card-document-link
[reopening from now closed https://github.com/hypothesis/h/pull/3550]