refactor/interpretations component [DHIS2-5470] #370

joakimia · 2018-12-07T12:02:23Z

This PR includes refactoring of the interpretations component, changes:

Added d2 as context on all components requiring access to the object, instead of passing it down as props. I've also removed imports on helper functions / models and are passing it through th fn parameter.
Added RichText and RichTextParser wrapper to the textareas.
Added Cancel Button permanently instead of only when editing an existing one.
Removed the InterpretationsDialog, instead the textarea is rendered at the bottom of the panel.
Reduced the renderfunctions by creating renderX fn.
Removed the misc.js file and utils/ folder and created folders for each function (So it wont become a dumping ground for all sorts of "generic" fn's / components.
I have removed the maxWidth on CollapsibleCard and added flex: '1 1 100% to the root container such that the component can use the available space in DV / Maps / Dashboard.
The Date, Likes and replies are rendered below the text of the interpretation/comment card.
If the created date is more than 24 hours old, the formatted relative date will be rendered instead of "x days/months/years ago".
Removed some unused code.
Re-structured/ Moved some test cases to new components that i have created.
Added Read/Write checks for interpretations.
Added prop Item and appName for the dashboard, in order to render the redirect button.
Added InputFields instead of Dialogs for original interpretations.
Added Toolbar with markdown functionality.
I did not put in the (edited) label, since the lastUpdated value changes when a user replies to the interpretation, or if the sharing properties are changed. Meaning that the (edited) label will eventually appear and might confuse the user (as it was supposed to indicate whether or not the specific text have been changed).
Bold / Italic is reset by the UI component in the new DV (I spoke with Viktor and he informed me that this component will be removed in the next release)

Quick Summary of the render structure:

InterpretationsCard:

Renders Either the interpretationsList, Or the specific Interpretation if View/Interpretaion is clicked
Renders the InputField, if the current interpretation have not been cliced/viewed and the user have write access to the analytic object.

InterpretationsList:

Renders all interpretations that the viewer have read access to. Renders 5 Interpretations along with a Show / Hide previous interpretations if the amount is higher than 5.

Interpretation:

Renders the Interpretation, its replies, (Sharing Dialog if "Manage Sharing" button is clicked, or Delete Dialog if Delete is Clicked).

ActionButtonContainer:

Renders the buttons depending on the user is either the owner, or have write access (There is no manage sharing dialog on comments, so it inherits the Interpretations sharing properties. If The user do not have write access, the person can not reply to the interpretations or comments, but can still read them).

ActionButton / Likes / Replies:

Render Icons / likes/ replies labels with 200ms onHover delay on tooltips

CommentsList:

Renders "View/Hide More replies" if the amount of comments is higher than 5.
Render the comments (Or NewCommentField if edit button is clicked).
Renders a NewCommentField at the bottom of the list if the user has write access

Comment:

Renders the comment, cardInfo and Buttons (if the user have write access / is owner of the comment)
Renders the Delete Dialog

NewInterpretationField/NewCommentField:

Renders the InputField with 3 states (initial, focused, focused with text present).
Renders the Toolbar with markdown and RichText wrapper.
Posts/Updates an Interpretation/Comment based on prop comment/interpretation
NewInterpretationField will render the sharing dialog when the "Manage Sharing" link have been added.

Toolbar:

Renders the button and Emoticon menu.
Uses markdown (I've mirrored the slack behavior in addition to the specification of the design document) file to insert markdown.
meta key + b/i triggers * or _

…leCard, replace Link text with InterpretationIcons, exporting Icons through misc.js, adding styles into styles file

… and Interpretations.js

…nd CommentTextArea.js, adding styles folder for components within interpretations/ folder. adding style file for CommentTextArea

…js wrapped with withStyles, removing unused className rules, adding PropType classes

… file for InterpretationComments.js changin in-line style to className, exporting and wrapping Component in withStyles

…='md' and will recieve rules from d2-ui-theme

…-line to classNames, adding PropTypes

….js in withStyles, adding style file

…sing style has its separate one

… all children in the same div

…ex all throughout the whole row

…ggestion, removing unnecesarry divs, replacing with fragments

…ncrease available space and mirror original interpretation structure

…r than 24 hours old, rendering specific date if evaluation returns true

…inor styling changes

… renders the tooltip above the icon when there's only 1 valid action rendered, minor adjustments after linking package locally and testing in DV

…ck-suggestion (momentarily) add new file NewInterpretation.js with boilerplate to replace the interpretation dialog that covers the whole screen, in order to not block the data you want to interpret

…ion.js instead of dialog

…lacing interpretations dialog, additional re-factoring

jenniferarnesen · 2018-12-10T08:24:28Z

packages/interpretations/src/components/interpretations/InterpretationsList.js

+    //TODO  length > 5 && showPreviousComments
+    // "Show previous comments  ( x  of < interpretations.length - rendered comments> )"
+    return (
+        interpretations.length ? (


Doesn't seem like this check is necessary, since interpretations.length was already checked on line 16

This file is not complete, the TODO comment might not be that clear as well. Currently i have a early return if there's no interpretation.
If there's more than 5 interpretations (lets say 15 of them), the component will mount with 10 hidden interpretations (with a "show previous comments" button), and render the 5 newest (mirroring the facebook/other social media behavior)

jenniferarnesen · 2018-12-10T08:26:30Z

packages/interpretations/src/components/interpretations/NewInterpretation.js

+            this.props.onClose();
+        });
+
+    _saveInterpretation() {


What does the leading "_" indicate?

Most of this file is copy pasted from InterpretationsDialog, i assume the underscore indicates that this function performs a network request.
I'll change the name to postInterpretation instead.

jenniferarnesen · 2018-12-10T08:31:09Z

packages/interpretations/src/components/styles/CollapsibelCard.style.js

@@ -0,0 +1,38 @@
+export const styles = theme => ({


mispelled filename:
CollapsibelCard -> CollapsibleCard

Corrected filename and import name of CollapsibleCard.

jenniferarnesen · 2018-12-10T08:31:31Z

packages/interpretations/src/components/styles/CollapsibelCard.style.js

+    expandOpen: {
+        transform: 'rotate(180deg)',
+    },
+});


Could you fix the missing newline on all the files?

Added newline on all the .style files. Checked the .style files in DV and I cant say that we are normally doing this, but the lines do get more clear. I think eslint/prettier removes it with the message "remove unnecessary newline"

jenniferarnesen · 2018-12-10T08:55:20Z

packages/interpretations/src/components/interpretations/Interpretation.js

 import i18n from '@dhis2/d2-i18n';
 import SharingDialog from '@dhis2/d2-ui-sharing-dialog';
+import { withStyles } from '@material-ui/core/styles';
 import some from 'lodash/fp/some';
 import InterpretationComments from './InterpretationComments';
 import InterpretationDialog from './InterpretationDialog';


InterpretationDialog file has been deleted.

I've re-created that file and added it back. I assume the symlink were still present and pointed to the inode which resided in my trash folder, so i didnt get any errors.

jenniferarnesen · 2019-02-18T13:58:24Z

packages/interpretations/src/components/Buttons/ActionButton.js

+		this.state = { tooltipIsOpen: false };
+	}
+
+	componentWillUnmount() {


Seems to be some alignment issues in this file?

jenniferarnesen · 2019-02-18T14:24:05Z

packages/interpretations/src/components/InterpretationsComponent/InterpretationsComponent.js

-                    onCurrentInterpretationChange={onCurrentInterpretationChange}
-                />
-            </div>
+            <MuiThemeProvider muiTheme={getMuiTheme()}>


What happens if we don't include this Provider?

jenniferarnesen · 2019-02-19T08:56:52Z

packages/interpretations/src/components/DetailsPanel/Details.js

@@ -60,7 +60,7 @@ export class Details extends React.Component {
        const SubscriptionButton = this.renderSubscriptionButton();

        return (
-            <CollapsibleCard title={i18n.t('Favorite details')}>
+            <CollapsibleCard title={i18n.t('Chart details')}>


It won't always be a chart. Is it possible to pass in the type (Map, Pivot table, chart...), or maybe it should just say "Details"? (double check with Joe).

…/interpretations-component

…ew interpretation when new model is loaded

…currentUser and pass to access validation checks

joakimia added 2 commits December 7, 2018 12:36

adding new file InterpetationIcon to replace text

0839c02

setting flex 100% on root div container, remove maxWidth on Collapsib…

058b0e3

…leCard, replace Link text with InterpretationIcons, exporting Icons through misc.js, adding styles into styles file

joakimia requested review from edoardo, amcgee and jenniferarnesen December 7, 2018 12:02

jenniferarnesen added the WIP label Dec 7, 2018

joakimia added 19 commits December 7, 2018 14:10

adding styles folder and separate styles files for CollapsibleCard.js…

bf88ae7

… and Interpretations.js

adding missing PropTypes for CollapsibleCard.js, Interpretations.js a…

9563b2a

…nd CommentTextArea.js, adding styles folder for components within interpretations/ folder. adding style file for CommentTextArea

changing styles from in-line to classNames, exporting Interpretation.…

3da6ee2

…js wrapped with withStyles, removing unused className rules, adding PropType classes

removing commented and unused code in Interpretation.js, adding style…

e218799

… file for InterpretationComments.js changin in-line style to className, exporting and wrapping Component in withStyles

removing width style rule on InterpretationDialog as this as maxWidth…

ebc2080

…='md' and will recieve rules from d2-ui-theme

adding style folder for InterpretationIcon.js, changing style from in…

1074f96

…-line to classNames, adding PropTypes

changing in-line style to classNames and wrapping InterpretationsCard…

9db9d00

….js in withStyles, adding style file

adding separate style folder for misc.js

99decd6

removing style fil interpretationsStyle.js now that every component u…

8ba7ed7

…sing style has its separate one

remove div separating user actions, replace with Fragment in order to…

399e585

… all children in the same div

setting marginRight percentages on flex container in order to not fl…

47e5bca

…ex all throughout the whole row

reordering icons and additional styling changes according to quick-su…

568dea9

…ggestion, removing unnecesarry divs, replacing with fragments

move created date of interpretation comments to top right corner to i…

4aa33fc

…ncrease available space and mirror original interpretation structure

additional styling adjustments, adding check to see if comment is ove…

ebd0b2e

…r than 24 hours old, rendering specific date if evaluation returns true

adding possibility to cancel new interpreation comments, additional m…

3a131f2

…inor styling changes

use break-word instead of break-all, set width on iconContainer so it…

91c9fdb

… renders the tooltip above the icon when there's only 1 valid action rendered, minor adjustments after linking package locally and testing in DV

change Icon spacing from flex to margin with spacing according to qui…

aedc3f9

…ck-suggestion (momentarily) add new file NewInterpretation.js with boilerplate to replace the interpretation dialog that covers the whole screen, in order to not block the data you want to interpret

add new files and separate components, use textArea in NewInterpretat…

43ad340

…ion.js instead of dialog

separating components for Description fields into separate files. rep…

f976e06

…lacing interpretations dialog, additional re-factoring

jenniferarnesen reviewed Dec 10, 2018

View reviewed changes

joakimia added 5 commits February 18, 2019 15:51

add missing whitespace at the end of file

b813d33

fix test cases

f56023a

remove MuiThemeProvider

387ac19

use itemTypeMap to dynamically render title for DetailsPanel

f9513c7

adjust test cases

79ce682

jenniferarnesen approved these changes Feb 19, 2019

View reviewed changes

joakimia added 22 commits February 19, 2019 12:37

Merge branch 'master' of https://github.com/dhis2/d2-ui into refactor…

3ea42d6

…/interpretations-component

add rich-text in package.json

adc93c0

correct import name

3c071c0

remove write access check on new interpretations

b74e6bf

add userAccess to Sharing text in details, update sharing props for n…

96e1864

…ew interpretation when new model is loaded

use prop type to render Details Title, fetch userGroupsId through d2.…

b647791

…currentUser and pass to access validation checks

use formatDate instead of formatRelative

fef3802

set write access to new interpretation if user have read access

cbdfa33

set initial sharing payload

8f1009c

enable writing interpretations to objects that have its owner removed

1d420ef

extract setting/getting sharing props to dedicated file

ada65be

get userGroup Ids with .keys() fn

4e632e2

adjust test-context

4631c74

styling for DetailsCard

97cbf9e

add toggle button to description

4329eee

styling for DetailsCard

f919a33

fix tests

792f88f

check read access when rendering current interpretation

7f9695d

styling

8d03d1f

render no interpretation label after filtering items

4ad1592

styling

961167a

add margin to back button

73111a3

joakimia merged commit 57e3633 into master Feb 26, 2019

joakimia deleted the refactor/interpretations-component branch February 26, 2019 09:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor/interpretations component [DHIS2-5470] #370

refactor/interpretations component [DHIS2-5470] #370

joakimia commented Dec 7, 2018 •

edited

Loading

jenniferarnesen Dec 10, 2018

joakimia Dec 11, 2018

jenniferarnesen Dec 10, 2018 •

edited

Loading

joakimia Dec 11, 2018

jenniferarnesen Dec 10, 2018

joakimia Dec 11, 2018

jenniferarnesen Dec 10, 2018

joakimia Dec 11, 2018

jenniferarnesen Dec 10, 2018

joakimia Dec 11, 2018

jenniferarnesen Feb 18, 2019

jenniferarnesen Feb 18, 2019

jenniferarnesen Feb 19, 2019

refactor/interpretations component [DHIS2-5470] #370

refactor/interpretations component [DHIS2-5470] #370

Conversation

joakimia commented Dec 7, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jenniferarnesen Dec 10, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joakimia commented Dec 7, 2018 •

edited

Loading

jenniferarnesen Dec 10, 2018 •

edited

Loading