Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

#162 Data Annotation #415

Merged
merged 47 commits into from
Jul 17, 2019
Merged

#162 Data Annotation #415

merged 47 commits into from
Jul 17, 2019

Conversation

hopetambala
Copy link
Contributor

Description of proposed changes
What does this pr accomplish and why should it be integrated?
Implement a note-taking feature that is a panel on the right-hand side that doubles as replacing the tooltip feature (found in d3). It has the ability to create a note with a title, labels, and note content. It then saves that note for a user to have the ability to export the notes and is retrievable on upload dataset.

Checklist
_Put an x in the boxes that apply. _

[x ]Lint and unit tests pass locally with my changes
[x ] I have added tests that prove my changes work (if applicable)
[x ]I have added or updated necessary documentation (if appropriate)

Additional comments
Is there anything else you want us to know?
Turns off the d3 mouseover

adds a setPosition action to the controls reducer (for a future use)

The tooltip prepopulates with a note if there is one as well
merge updated dependencies
@hopetambala hopetambala requested a review from rashley-iqt July 12, 2019 16:36
@hopetambala hopetambala changed the title #162Data Annotation #162 Data Annotation Jul 12, 2019
@codecov
Copy link

codecov bot commented Jul 16, 2019

Codecov Report

Merging #415 into master will increase coverage by 0.17%.
The diff coverage is 89.28%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #415      +/-   ##
=========================================
+ Coverage   82.62%   82.8%   +0.17%     
=========================================
  Files          28      29       +1     
  Lines         823     849      +26     
=========================================
+ Hits          680     703      +23     
- Misses        143     146       +3
Impacted Files Coverage Δ
src/epics/load-dataset-epic.js 91.46% <100%> (+0.21%) ⬆️
src/epics/upload-dataset-epic.js 94.28% <100%> (+0.34%) ⬆️
src/domain/controls.js 93.93% <71.42%> (-6.07%) ⬇️
src/domain/notes.js 93.33% <93.33%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 996df73...edcbc54. Read the comment docs.

src/domain/notes.js Show resolved Hide resolved
src/App.js Show resolved Hide resolved
src/domain/controls.js Outdated Show resolved Hide resolved
src/domain/notes.js Outdated Show resolved Hide resolved
src/features/tooltip/Tooltip.js Outdated Show resolved Hide resolved
src/features/tooltip/Tooltip.js Outdated Show resolved Hide resolved
src/features/visualization/d3-viz.js Outdated Show resolved Hide resolved
src/features/visualization/d3-viz.js Outdated Show resolved Hide resolved
src/features/visualization/d3-viz/setup-tooltip.js Outdated Show resolved Hide resolved
@hopetambala hopetambala requested a review from rashley-iqt July 16, 2019 16:14
Copy link
Member

@rashley-iqt rashley-iqt left a comment

Choose a reason for hiding this comment

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

  • Please correct linting errors as they are causing CI to fail.
  • I think it would be better to hide the detail panel on the initial load until there is something to show the details of.
  • I think the save/delete buttons should be:
    • centered
    • black and white
    • consistent with the size/style of the other icons
  • Visually offset the notes area from the imported data, maybe something like a horizontal rule?
  • Once a note is entered, a user has to click off of the node and then back on to see the note.
  • Please make it visually clear that the title and note areas are text areas are text fields. Try to use the same style as other text fields
  • make the notes entry area fit the width of the tooltip and, if possible, multi-line
  • the tagging feature doesn't seem to work in any expected way. Hide this for now and we will add this in a subsequent PR.

- make the notes entry area fit the width of the tooltip and multi-line
- removed tagging feature
-  hide the detail panel on the initial load until there is something to show the details of
- make tooltip adhere to CRViz style
@hopetambala hopetambala requested a review from rashley-iqt July 17, 2019 15:39
@hopetambala
Copy link
Contributor Author

hopetambala commented Jul 17, 2019

Addressed these issues

  • Linting errors
  • Hid the detail panel on the initial load until there is something to show the details of.
  • Save/delete buttons are:
    • centered
    • black and white
    • consistent with the size/style of the other icons
  • Visually offset the notes area from the imported data with CRViz style conventions
  • Once a note is entered, a user doesn't have to click off of the node and then back on to see the note.
  • Made visually clear that the title and note areas are text fields
  • Made the notes entry area fit the width of the tooltip through multi-line
  • Remove Tagging feature

@rashley-iqt rashley-iqt merged commit 848f4ad into IQTLabs:master Jul 17, 2019
hopetambala added a commit to hopetambala/CRviz that referenced this pull request Jul 18, 2019
feat(annotation): Merge pull request IQTLabs#415 from hopetambala/master
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants