[Feature]: Comments and Annotations.

[digi0ps]

Modules

• src/sidebar-overlay:
  Contains:

  • presentational, conatiner components of Sidebar and Ribbon
  • content_script for Ribbon, iFrame injection
  • Redux code for sidebar

• src/overview/sidebar: Contains redux code related to overview sidebar.

• src/direct-linking/background/storage.ts - Storage ops related to CAH.

External dependencies

• react-burger-menu
• moment
• dexie

TODO

Sidebar

• New annotation component to display highlights
• Display highlight with annotation
• Truncate both highlight and annotation with toggle
• Edit annotation UI with toggle
• Footer state "default": Display icons
• Footer state "edit": Display cancel and Save
• Footer state "delete": Display Yes and cancel
• Use moment to display timestamp
• Display tag pills
• Fix sidebar not scrolling issue
• Update CommentBox with new design
• Add tag input to commentBox
• Go to webpage when the button is clicked
• Separate logic and presentation in Annotation component

Sidebar in websites

• Move sidebar related components to a separate module
• Add new webpack entry for sidebar.html
• Add styles for sidebar.html
• Create Ribbon component
• Inject Ribbon and iframe through content script
• Unify logic of Sidebar. ( Currently all the logic of sidebar are existing in src/overview/sidebar and src/sidebar-overlay/container)

Storage

• Rebase feature/direct-linking-ui for StorageManager
• Add methods to store/retrieve annotations in StorageManager
• Hook up storage methods in UI
• Figure out a way to store tags

Bugs

• Set pageTitle properly for comments. (Right now it's being set as the overview page's title. A better way would be to add pageUrl, pageTitle to state to easily access it.)
• createdWhen is unreadable in the client, need to fix this to sort annotations.
• Reload sidebar overlay state on every update. ( Have to refresh right now )
• In sidebar overview, clicking another comment icon when the sidebar is already open doesn't update the sidebar.
• Close sidebar when pressed outside.

[blackforestboi]

Great work :)
Really can see how it is working soon!

Some things I noticed and leaving here for documentation:

• When scrolling in the sidebar, can we somehow prevent scrolling in the overview or the whole screen happening. Now, when scrolling is finished in the sidebar it continues in the overview.
• Adding a tag field to the comment input as well (forgot about that when making the mockups)
• Making sure that you can scroll to the real bottom > and leave a bit of space underneath the last element, so it does not look like crammed.

[digi0ps]

Thanks. :)
That scrolling bug is already in the Todo, something to do with react-burger-menu, will have to dig through their repo.
Will add the other two!

[blackforestboi]

will have to dig through their repo.

I think @subrat-sahu ran into the same issue, right?

[subrat-sahu]

@oliversauter @digi0ps
Haven't addressed the issue of scrolling, but the third point i.e.

Making sure that you can scroll to the real bottom > and leave a bit of space underneath the last element, so it does not look like crammed

And the scrolling behaviour I think is not specific to react-burger-menu it is common.

[digi0ps]

And the scrolling behaviour I think is not specific to react-burger-menu it is common.

Yup sorry, it didn't strike then!

[blackforestboi]

@poltak @digi0ps

RE: the behaviour for deleting/updating annotations.

User story:

As a user I want to change the body content of a comment/annotation/highlight with the changes being searchable afterwards. I don't expect the old words to still be searchable after a change.

[blackforestboi]

Great stuff, so happy to see this working :) Well well well done @digi0ps

A couple of things that need some repolishing:

• When you create a second annotations on the same page, it creates this glitch:
• The input fields should be expandable (so that people can manually drag them longer (not wider)).
• nytimes is still above memex stuff:
• opening an annotation from the overview sidebar results in a "file not found"

[digi0ps]

Fixed 1 and 3.
2) The tooltip keeps expanding vertically by one row as the user types right now. I will make it draggable too.
4) Ah, yeah, cause highlights/annotations don't have an redirect-able URL. I will fix this!

[blackforestboi]

Another few improvements:

• When a new annoation is created and the sidebar opens, it should show the other highlights (just as if you would open only the sidebar)
• When adding a new annotations, all tags are already selected. They shouldn't.
• The new design for adding tags is included in the document I sent you on slack
• Make sure the new lines inserted into a comment are preserved in the saved card. It looks weird when a structure is crammed into a single flowing text.
• sometimes pages load sooo long that it takes forever for the sidebar to be loadable. When I press 'esc' to cancel the loading process (because it is mostly only the ads loading, text is already there) I want to get the sidebar immediately.
• When I click on a highlighted piece of text when the sidebar is open, I want to jump to the annotation in the sidebar. Now it is closing the sidebar.
• When creating a direct link, the sidebar should not open

[blackforestboi]

Another few improvements:

• sometimes when closing the sidebar in the overview, the scroll-stop is still enabled, and I cant - [ ] scroll in the results anymore.
• Somehow the color is really dark green again.
• When you click on an annotation and the sidebar opens, the annotation you are currently working with should have a slight darker green than the regular highlight one.
• When you click on the "go to annotation" in overview sidebar, it should scroll (it does) and open the sidebar with the correct annotation being shown (doesn't). Showing that annotatoin should happen like in the mockups, where the shadow is bigger and green.

[blackforestboi]

good morning @digi0ps :)

I checked on your PR. We are getting there but there is still a lot of work to do to round it up.
Re the problem with the closing icon, I attached a new PNG here with an update to the sidebar.

In general, there is just still a lot of work to do to fit it to the designs of the annotations, try to be as closely as possible to the current designs, and if you have questions, I am always there :)
Specifically font, and spacings still need some work. Also the overview sidebar looks pretty different to the in-page sidebar. The IBM plexi font is meant for the quote, not the date by the way :)
Also all the points above that have not been tackled yet, please finish them before the next review.

We are almost there :) Congrats!

[ShishKabab]

Just as only, or as single?

[ShishKabab]

Great and lots of work! Cool that you've got this feature almost ready! Reading through the code, I've come up with the follow things:

• I see you've extended the direct linking module. Since I'd like to keep naming clean, there's a few options. Either we move everything to src/annotations that has to do with storage, etc. This is because we could view a direct link as an annotation, but not every annotation is a direct link. On the other hand, since the direct linking functionality is quite seperate for now, I'd also just move them to a different collection that can evolve indepently. This way we keep things separated and clean. In the future, if the UI requirements really merge these two in terms of functionality, we can always easily merge the data.
• Also for the other things, you can move things like scrollToHighlight to src/annotations/ and import them from there if you want, since direct links build on top of annotations, not the other way around.
• Right now there are two CSS classes memex-highlight(-dark) which are used for 1) styling and 2) finding the position again. I'd use memex-highlight for every highlight so you can find them again, and attach a second memex-highlight-(dark|light) for styling it correctly.
• There are still some monster components (AnnotationContainer, Annotation) around, judging by lines of code and amount of properties they take. Components that are getting just a bit too big. Maybe you could split them into smaller stateless components. You could also split out the function that do not touch this. to small functions. That way you can shorten the code in the container to see more clearly what is happening in which order.
• Instead of passing around rendering functions, I'd say only pass data around, and maybe component classes of you want to customize rendering from outside. Calling back from deeper components creates complicated call graphs, which make the program more difficult to comprehend and debug.
• Currently I'm a bit confused about what our policy about Redux vs. component state. When I make React/Redux applications, I store almost everything in Redux (except where performance doesn't allow) and calculate all the rest on the fly in pure component/functions. Here stuff seems to be mixed a lot. What do you think @poltak? Also interaction with data sources (like Ribbon.fetchAnnotations) I'd like to somehow see separated from the rendering code.

[poltak]

Currently I'm a bit confused about what our policy about Redux vs. component state. When I make React/Redux applications, I store almost everything in Redux (except where performance doesn't allow) and calculate all the rest on the fly in pure component/functions. Here stuff seems to be mixed a lot. What do you think @poltak?

The main thing we've tried to encourage here is making the actual underlying state (what's in the store) as simple as possible, then putting the majority of the logic needed to produce the derived states (what's used in containers/views) in memoized selectors.
In other words: don't store anything that can be derived (unless there's an edge-case where the derivation is sufficiently complex in terms of space+time - although most of these cases can be avoided with the right data structure).

For maintainability, and especially flexibility with accessing states/triggering actions from different parts of the codebase, it would be really nice to put everything in redux and ignore React component state. But redux does come with an large amount of boilerplate which can be especially annoying if the state is super simple and probably only ever going to be used in the context of that component 😕

Also interaction with data sources (like Ribbon.fetchAnnotations) I'd like to somehow see separated from the rendering code.

Interaction with data sources should definitely be separated from the rendering code. At the very least there should be a clear distinction between container and view/presentational components (sometimes called "dumb" and "smart" components, and prob a bunch of other names), with data interaction being done at the container level, and/or in redux thunks - esp. if an explicit React container class is not needed.
(I presume "rendering code" here means views specifically, so not including container components?)

Probably already acknowledged by everyone, but also note interaction with the main data (stuff that is stored in StorageManager) should only ever be accessible from the background script. There will need to be some interscript communication method being employed to get it from the BG script to the options UI script (so the utils/webextensionRPC module - or using the runtime.send/onMessage APIs directly if really needed). These "remote" calls to trigger data interaction in the BG script have generally been done at the container/thunk level.

[ShishKabab]

I presume "rendering code" here means views specifically, so not including container components?

Exactly :) If any state at all is stored in React components or interactions with the back-end, I'd do it in container components, not mixed inside presentation components.

[blackforestboi]

@digi0ps Great work, almost almost there!

What still needs some doing:

Before Launch

• When user clicks on "annotation" icon in the tooltip, it should just immediately open the sidebar, and remove the tooltip. No state change of the tooltip.
• while loop-bug
• The comment and tag field, and the save and cancel buttons should have font type lato. The comments and tags field is italic. Actually everything is lato except the annotation text. The save and cancel button are not lato when in editing mode as they are in standard "add mode"
• Tags should behave and look consistent across the tool, so let it look like, and use the exact fonts used in the popup tag adder. Also for the "X tags selected. Also make the tags selectable with keyboard access only (up, down, enter to select).
• Change the top bar design of the sidebar to this. (ignore the rest of the screenshot, its just about the topbar) Grey out the "add comment" button, if one is already in the making. color of border D0CECE. Use the X (and its color) of the ribbon for closing icon, make #3eb995 when hovering and show grey circle around it.
• Remove the box with "0 tags selected" when clicking on "add tag"
• make the tag pills in the color #3eb995, and the text inside white
• When a user hovers over an annotation, it should also make the card in the sidbar go in hover state. also the color of the highlight should get a bit darker. Same goes for when hovering over a card, it should make the highlight darker.
• The text below a highlight is not selectable anymore. Once you select it, the blue system selection just disappears again.
• scrolling in the in-page sidebar should not scroll the page itself, only the sidebar.
• Remove "all notes" separator, Its actually useless at this point.
• Sort annotations by their order of appearing on the page, make sure after saving an annotation, it highlights the annotation currently available.
• when user only adds a tag to an annotation, display it. Right now nothing is shown.
• Make border of sidebar: e1ece9
• This is what it looks like when you edit a comment. Also remove the dashed line
• Change icons in tooltip/hover state: Here the icons for the tooltip icons. empty space for inactive, filled space for onhover: Archive.zip
• the comment box is not draggable, it just shows the bottom right side to drag
• The comment should also have a show more/less arrow.
• the ribbon should be visible and the sidebar should be openable before the page fully loaded.
• when writing a comment, then pressing 'esc' it just removes everything in the sidebar (except the close button). Nothing should happen.

CAN be after launch, but should not:

• When user makes annotation, the typing cursoer already should be in the comment field, so the user can start typing right away.
• Make it possible to switch between the fields with "tab". So first comment field, then tag, then save button. If people are currently having the tag adder opened, and press tab, it should close the tag adder list and jump to the save button.
• Letter spacing for the "annotation text" > the IBM font one > 0.3px
• remove the dashed line entirely, also not solid.
• When first selecting a page with annotations already available, then opening another page, it shows the old annotation for a second, which it should not. (Video)
• Swap the position of the X and the settings icon in the ribbon
  - [ ] Make the sidebar open faster

[blackforestboi]

@digi0ps Ah looks already really good.

A couple of things caught my eye that still need doing:

Priority 1:

• This is how it looks to edit an annotation now:
• In the top bar, it looks kind of weird to open it because the closing icon pops up weirdly. How did hypothes.is do it exactly? It looks super smooth with them. Video
• When an annotation is about to be made, still show "Add new annotation" as a description. Also disable the "add comment" button in this case
• remove that huge space between the separator and the comment. This is how it should look like: This is how it looks now:

Priority 2:

• If someone highlighted a piece of text and the page is not finished loading, it should afterwards show the tooltip, without the user having to select the place freshly.

[blackforestboi]

A few things i recognised:

• This:
• Deleting tag pills does not work
• when adding a new comment and then searching and adding a tag with enter, lets the cursor jump back into the text field.
• This: http://recordit.co/aedMtH5lEw

[blackforestboi]

Another few things:

• there is this little X popping up: http://recordit.co/3GQzvRKu9U
• the ribbon, and the sidebar, only become available after the 3rd page you open. or if you reload the first page you open

[digi0ps]

the ribbon, and the sidebar, only become available after the 3rd page you open. or if you reload the first page you open

Can you tell me more?

[blackforestboi]

1. install memex freshly
2. visit first site, ribbon does not pop up, try annotate, nothing happens.
3. visit 2nd site, ribbon does not pop up
4. visit 3rd site, ribbon pops up and you can also annotate

[blackforestboi]

Another few bugs:

EDIT:

• maybe related to the bug i mentioned on slack: When you are on a page that you opened with a Memex.Link, and then select another annotation from the sidebar, it scrolls there, then back to the position you have been before clicking on the anntoation

[digi0ps]

@ShishKabab @poltak

Sorry for the late response, I was working on fixing the bugs. But I was worked on some of the things you said side by side.

Moved most of the interactions functions which are related to higlights to the sidebar module, unified the highlight classes into one, got rid of most of the render-* functions sending data around instead. Working on making AnnotationContainer smaller.

Either we move everything to src/annotations 

This seems perfect, I will move all the annotation storage related backend to a separate module. (src/annotations)

Currently I'm a bit confused about what our policy about Redux vs. component state.

I'm in the process of moving the CommentBox states to the Redux. I set up React states for AnnotationContainer because it needs separate states for each annotation box (for truncation, footer etc.). This might require a separate Redux Store which is different from the Sidebar's Redux store (correct me if I'm wrong), so I thought having React state for these is better!

Also interaction with data sources (like Ribbon.fetchAnnotations) I'd like to somehow see separated from the rendering code.

Regarding Ribbon.fetchAnnotations, it just calls the remote function "fetchAnnotations" and sets the Ribbon state with the fetched annotation, there is no direct calls to the browser storage involved.

Right now the data flow is different for in-page sidebar/overview sidebar. In the overview sidebar it's simple, it's fetched through remote calls. While in the in-page sidebar, the annotations are fetched in the Ribbon, highlighted sorted and then passed to the sidebar.html ( to make the annotations sorted by their position in the page. )