-
Notifications
You must be signed in to change notification settings - Fork 99
(feat) O3-3914: Support re-arranging concept answers to preferred order #403
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.
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
base: main
Are you sure you want to change the base?
Conversation
@NethmiRodrigo and @denniskigen needs review..! |
Hey @NethmiRodrigo, could you please clarify if we should implement a cloning approach so that the original answer item remains visible in the list while dragging, or is it acceptable that the item is temporarily removed from its position and only appears in the drag layer until dropped? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Bharath-K-Shetty! I know this is a large ask, but is it possible to reuse the functionality that we have that enables dragging questions in the interactive builder:
It's okay if we would have to change the appearance of the answers from the blue tags to something like above. Its best if we can minimize the use of third party libraries. We already use a couple and I'd like to minimize the use as much as possible to keep the build dize small.
10ee48e
to
40e8340
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Bharath-K-Shetty! Getting close, just a few more adjustments
import type { Concept } from '@types'; | ||
import styles from './select-answers.scss'; | ||
|
||
import { DndContext } from '@dnd-kit/core'; | ||
import type { DragEndEvent } from '@dnd-kit/core'; | ||
import { SortableContext, useSortable, arrayMove, verticalListSortingStrategy } from '@dnd-kit/sortable'; | ||
import { CSS } from '@dnd-kit/utilities'; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please cleanup the import order - https://o3-docs.openmrs.org/docs/coding-conventions/code-organization:
Group imports alphabetically based on their type. The recommended order is:
React and framework imports (e.g., React, useState, useEffect)
External modules (e.g., lodash, dayjs, react-i8next)
Carbon component imports (e.g., Button, InlineLoading)
OpenMRS imports (e.g., @openmrs/esm-framework, @openmrs/esm-patient-common-lib)
Local imports (components, hooks, utilities, etc.)
Asset imports (e.g import styles from './user.scss')
interface SortableTagProps { | ||
answer: AnswerItem; | ||
} | ||
|
||
function SortableTag({ answer }: SortableTagProps) { | ||
const { attributes, listeners, setNodeRef, transform, transition, isDragging } = useSortable({ | ||
id: answer.id, | ||
}); | ||
|
||
const dynamicStyle = { | ||
transform: CSS.Transform.toString(transform), | ||
transition, | ||
}; | ||
|
||
return ( | ||
<div ref={setNodeRef} style={dynamicStyle} {...attributes} {...listeners}> | ||
<Tag className={`${styles.sortableTag} ${isDragging ? styles.dragging : ''}`} type="blue"> | ||
{answer.text} | ||
</Tag> | ||
</div> | ||
); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is a separate component, let's have it in its own file. There should be a common
folder in the question
folder - https://github.com/openmrs/openmrs-esm-form-builder/tree/main/src/components/interactive-builder/modals/question/question-form/common. We could create a sub folder for this component there
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok @NethmiRodrigo got it..!
40e8340
to
4573270
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Bharath-K-Shetty, this is very well done! A few adjustments to make.
- If a user has additional answers added, ordering the answers causes the additional answers to appear in the obs answers section.
Screen.Recording.2025-03-12.at.2.22.02.PM.mov
The context behind this is, a concept can have its own answers, which a user can see in the first dropdown to select answers. If a concept is of type Coded
, a user can search for and add other additional concepts as answers.
These additional answers are shown separate when creating a question so that they can search or remove.
2. We should support ordering for the additional answers.
3. It would help to have an icon in the tag (like we do for questions) to let the user know that it can be dragged
…extended DragNDrop for additional concept answers
dbd42b9
to
485ea14
Compare
import React, { useCallback, useEffect, useMemo, useState } from 'react'; | ||
|
||
import { Tag, MultiSelect, Stack } from '@carbon/react'; | ||
import { DndContext, type DragEndEvent } from '@dnd-kit/core'; | ||
import { arrayMove, SortableContext, verticalListSortingStrategy } from '@dnd-kit/sortable'; | ||
import { useTranslation } from 'react-i18next'; | ||
import ConceptSearch from '../../../common/concept-search/concept-search.component'; | ||
import { useFormField } from '../../../../form-field-context'; | ||
|
||
import type { Concept } from '@types'; | ||
import { useFormField } from '../../../../form-field-context'; | ||
import ConceptSearch from '../../../common/concept-search/concept-search.component'; | ||
import { SortableTag } from '../../../common/sortable-tag/sortable-tag.component'; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please re-order the imports to match standards - https://o3-docs.openmrs.org/docs/coding-conventions/code-organization
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const saveQuestion = useCallback(() => { | ||
const current = formField.questionOptions?.answers || []; | ||
const mergedAnswers = [ | ||
...current, | ||
...additionalAnswers | ||
.filter((ans) => !current.some((a) => a.concept === ans.id)) | ||
.map((ans) => ({ concept: ans.id, label: ans.text })), | ||
]; | ||
const updatedFormField: FormField = { | ||
...formField, | ||
questionOptions: { | ||
...formField.questionOptions, | ||
answers: mergedAnswers, | ||
}, | ||
}; | ||
|
||
setFormField(updatedFormField); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are the additional answers added into the context? Seems a bit unnecessary to do this on the modal. The whole point of breaking down into components was so that the main component, in this case the question modal, wouldn't have to worry about things like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @NethmiRodrigo As much as i know and correct me if I am wrong,It is because that previously when we were adding additional concept answers that were stored as a separate array of that and were merging when setFormField is triggered and it was triggering when we click on save.After the drag and drop is introduced after adding additional concept answers and when we try to drag the concept answers at that moment only it was merging because it was triggering the setFormField.So what I did is that I explictely made the merging should happen when user click save until that time it should be separated.Is there any better approach for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the concept answer tags are rendered from an array called selectedAnswers
here -
Lines 25 to 32 in cb4de45
const selectedAnswers = useMemo( | |
() => | |
formField.questionOptions?.answers?.map((answer) => ({ | |
id: answer.concept, | |
text: answer.label, | |
})) ?? [], | |
[formField.questionOptions?.answers], | |
); |
If the
addedAnswers
state is not empty, we can just filter out the elements in addedAnswers
from selectedAnswers
. That way, we wouldn't have to do so much.
<button | ||
className={styles.conceptAnswerButton} | ||
onClick={() => handleDeleteAdditionalAnswer(answer.id)} | ||
> | ||
X | ||
</button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By using a sortable tag, we've lost the feature to remove an added answer, which shouldn't be the case
Requirements
Summary
This PR introduces drag-and-drop functionality to the Select Answers component in the form builder, enabling users to rearrange their selected answers to match their preferred display order.
Key changes include:
Screenshots
Screen.Recording.2025-02-28.145937.mp4
Related Issue
https://openmrs.atlassian.net/browse/O3-3914
Other