Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: add general comment field to analysis section of any element #2241

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

feat: add general comment field to analysis section of any element #2241

wants to merge 4 commits into from

Conversation

adambasha0
Copy link
Contributor

@adambasha0 adambasha0 commented Nov 16, 2024 •
edited by PiTrem
Loading

allow user to add a general comment on the analyses section of any element (not related to commenting function on shared elements)

@github-actions GitHub Actions
Copy link

LCOV of commit 45e0515 during Continuous Integration #4153

Summary coverage rate:
  lines......: 65.5% (14037 of 21436 lines)
  functions..: no data found
  branches...: no data found

Files changed coverage rate: n/a

@adambasha0 adambasha0 self-assigned this Nov 19, 2024
@github-actions GitHub Actions
Copy link

LCOV of commit acafbea during Continuous Integration #4159

Summary coverage rate:
  lines......: 65.5% (14037 of 21436 lines)
  functions..: no data found
  branches...: no data found

Files changed coverage rate: n/a

@adambasha0 adambasha0 requested a review from haditariq November 19, 2024 13:29
@adambasha0
Copy link
Contributor Author

Chemotion.dev.2.webm

haditariq
haditariq reviewed Nov 26, 2024
View reviewed changes
app/packs/src/components/common/AnalysisCommentBoxComponent.js
@@ -0,0 +1,55 @@
import React from 'react';
Copy link
Contributor

Choose a reason for hiding this comment

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

1- Analysis component should not be in common componenets foler. I think calling it a Button would be more fitting.
2- Exporting 2 indiviual components from one file is not a standard for reusable components.
3- Building a wrapper or individual files would be a nice option.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think common folder is suitable for reusable components. Both are closely related, therefore in one file. Building a wrapper that gather both components does not fit here, because both components are called in different positions.

haditariq
haditariq requested changes Nov 26, 2024
View reviewed changes
app/packs/src/apps/mydb/elements/details/cellLines/analysesTab/AnalysesContainer.js
import ElementStore from 'src/stores/alt/stores/ElementStore';
import OrderModeRow from 'src/apps/mydb/elements/details/cellLines/analysesTab/OrderModeRow';
import EditModeRow from 'src/apps/mydb/elements/details/cellLines/analysesTab/EditModeRow';
import PropTypes from 'prop-types';
import { CommentButton, CommentBox } from 'src/components/common/AnalysisCommentBoxComponent';
Copy link
Contributor

Choose a reason for hiding this comment

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

Refactoring requested

app/packs/src/components/common/AnalysisCommentBoxComponent.js
variant="primary"
onClick={toggleCommentBox}
>
Add comment
Copy link
Contributor

Choose a reason for hiding this comment

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

If its a toggle button it should convey user through UI.

app/packs/src/apps/mydb/elements/details/cellLines/analysesTab/AnalysesContainer.js

handleCommentTextChange = (e) => {
const { currentElement } = ElementStore.getState();
currentElement.container.description = e.target.value;
Copy link
Contributor

Choose a reason for hiding this comment

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

  • Is description exists always?
  • Checking for valid value before saving and triming the value?

Copy link
Contributor Author

@adambasha0 adambasha0 Jan 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

yes, description always exists.

app/packs/src/apps/mydb/elements/details/reactions/analysesTab/ReactionDetailsContainers.js
@@ -234,8 +241,18 @@ export default class ReactionDetailsContainers extends Component {
return null;
}

handleCommentTextChange = (e) => {
const { reaction } = this.props;
reaction.container.description = e.target.value;
Copy link
Contributor

Choose a reason for hiding this comment

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

Checking value is valid and trimmed before saving

app/packs/src/components/common/AnalysisCommentBoxComponent.js
function CommentBox({ isVisible, value, handleCommentTextChange }) {
return isVisible && (
<Form.Group>
<Form.Control
Copy link
Contributor

Choose a reason for hiding this comment

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

Optional: Have a placeholder for textarea would look nice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

PiTrem added a commit that referenced this pull request Dec 18, 2024
@PiTrem
feat: extend comment box field in analysis tabs for other elements #2241
252a60d 


Squashed commit of the following:

commit acafbea
Author: adambasha0 <adambasha1992@gmail.com>
Date:   Tue Nov 19 12:52:55 2024 +0000

    refactor: add unit tests for AnalysisCommentBoxComponent

commit 45e0515
Author: adambasha0 <adambasha1992@gmail.com>
Date:   Fri Nov 15 14:46:57 2024 +0000

    refactor: comment box and comment button components for analysis tabs of reactions

commit 12df07d
Author: adambasha0 <adambasha1992@gmail.com>
Date:   Fri Nov 15 12:28:53 2024 +0000

    feat: add comment box and comment button in analysis tabs for elements
PiTrem added a commit that referenced this pull request Dec 18, 2024
@PiTrem
feat: extend comment box field in analysis tabs for other elements #2241
7cc1a1d 


Squashed commit of the following:

commit acafbea
Author: adambasha0 <adambasha1992@gmail.com>
Date:   Tue Nov 19 12:52:55 2024 +0000

    refactor: add unit tests for AnalysisCommentBoxComponent

commit 45e0515
Author: adambasha0 <adambasha1992@gmail.com>
Date:   Fri Nov 15 14:46:57 2024 +0000

    refactor: comment box and comment button components for analysis tabs of reactions

commit 12df07d
Author: adambasha0 <adambasha1992@gmail.com>
Date:   Fri Nov 15 12:28:53 2024 +0000

    feat: add comment box and comment button in analysis tabs for elements
@PiTrem PiTrem changed the title Extend comment box field in analysis tabs for other elements feat: extend comment field to analysis tabs for other elements Jan 8, 2025
PiTrem added a commit that referenced this pull request Jan 10, 2025
@PiTrem
feat: extend comment box field in analysis tabs for other elements #2241
2fb68fd 


Squashed commit of the following:

commit acafbea
Author: adambasha0 <adambasha1992@gmail.com>
Date:   Tue Nov 19 12:52:55 2024 +0000

    refactor: add unit tests for AnalysisCommentBoxComponent

commit 45e0515
Author: adambasha0 <adambasha1992@gmail.com>
Date:   Fri Nov 15 14:46:57 2024 +0000

    refactor: comment box and comment button components for analysis tabs of reactions

commit 12df07d
Author: adambasha0 <adambasha1992@gmail.com>
Date:   Fri Nov 15 12:28:53 2024 +0000

    feat: add comment box and comment button in analysis tabs for elements
@PiTrem PiTrem changed the title feat: extend comment field to analysis tabs for other elements feat: add general comment field to analysis section of any element Jan 10, 2025
@PiTrem PiTrem added this to the v2.0.0 milestone Jan 21, 2025
PiTrem added a commit that referenced this pull request Jan 21, 2025
@PiTrem
feat: extend comment box field in analysis tabs for other elements #2241
e66f9b7 


Squashed commit of the following:

commit acafbea
Author: adambasha0 <adambasha1992@gmail.com>
Date:   Tue Nov 19 12:52:55 2024 +0000

    refactor: add unit tests for AnalysisCommentBoxComponent

commit 45e0515
Author: adambasha0 <adambasha1992@gmail.com>
Date:   Fri Nov 15 14:46:57 2024 +0000

    refactor: comment box and comment button components for analysis tabs of reactions

commit 12df07d
Author: adambasha0 <adambasha1992@gmail.com>
Date:   Fri Nov 15 12:28:53 2024 +0000

    feat: add comment box and comment button in analysis tabs for elements
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@haditariq haditariq haditariq requested changes

Requested changes must be addressed to merge this pull request.

Assignees

@adambasha0 adambasha0

Labels
None yet
Projects
None yet
Milestone
v2.0.0
Development

Successfully merging this pull request may close these issues.
3 participants
@adambasha0 @haditariq @PiTrem