-
Notifications
You must be signed in to change notification settings - Fork 82
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
(refactor) O3-4145: Break question modal into individual components #359
Open
NethmiRodrigo
wants to merge
29
commits into
main
Choose a base branch
from
refactor/question-modal
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Size Change: +7.52 kB (+0.19%) Total Size: 3.9 MB
ℹ️ View Unchanged
|
NethmiRodrigo
force-pushed
the
refactor/question-modal
branch
7 times, most recently
from
November 29, 2024 11:23
79f4893
to
3e36049
Compare
NethmiRodrigo
force-pushed
the
refactor/question-modal
branch
2 times, most recently
from
December 9, 2024 16:36
96f7b4c
to
a111ba5
Compare
NethmiRodrigo
force-pushed
the
refactor/question-modal
branch
from
December 19, 2024 08:04
7a2c23e
to
d76aad3
Compare
ibacher
reviewed
Dec 19, 2024
src/constants.ts
Outdated
Comment on lines
3
to
14
export const questionTypes = [ | ||
'control', | ||
'encounterDatetime', | ||
'encounterLocation', | ||
'encounterProvider', | ||
'encounterRole', | ||
'obs', | ||
'obsGroup', | ||
'patientIdentifier', | ||
'testOrder', | ||
'programState', | ||
]; |
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.
You can get better types this way, I think:
export const questionTypes = [
'control',
'encounterDatetime',
'encounterLocation',
'encounterProvider',
'encounterRole',
'obs',
'obsGroup',
'patientIdentifier',
'testOrder',
'programState',
] as const;
export type QuestionType = typeof questionTypes[number];
export const renderTypeOptions: Record<Exclude<QuestionType, 'obs'>, Array<RenderType>> = {
control: ['text', 'markdown'],
encounterDatetime: ['date', 'datetime'],
encounterLocation: ['ui-select-extended'],
encounterProvider: ['ui-select-extended'],
encounterRole: ['ui-select-extended'],
obsGroup: ['group', 'repeating'],
testOrder: ['group', 'repeating'],
patientIdentifier: ['text'],
programState: ['select'],
};
Now QuestionType
can be used where you need to reference a member of questionTypes
and will be kept in sync with that list.
NethmiRodrigo
force-pushed
the
refactor/question-modal
branch
from
January 8, 2025 10:44
ff38986
to
a79e1a3
Compare
ibacher
approved these changes
Jan 9, 2025
NethmiRodrigo
force-pushed
the
refactor/question-modal
branch
from
January 10, 2025 06:42
594195c
to
cf23597
Compare
NethmiRodrigo
force-pushed
the
refactor/question-modal
branch
from
January 17, 2025 07:07
10b842f
to
c80da17
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Requirements
Summary
This PR aims to refactor the question modal and organize the code structure.
1. Refactor to the question modal
Why we need this?
The existing implementation had a major problem - there were two separate components (AddQuestionModal and EditQuestionModal) to support creating and editing a question. This meant that
Solution implemented
For both creating and editing a question, the same question modal will be used. All components introduced have at least basic unit tests introduced.
Question
component. If the main question is anObsGroup
, it will render an option to create a grouped question that will render theQuestion
component again.Adding support for ObsGroup wasn't meant to be a part of the refactor, but I included it to make sure that the refactor was done in a way that would support this
RenderingType
component andQuestionType
component.QuestionType
- each question type has its own component (e.g - obs) . This makes it easier to add support to new question types without making a singular component too big or complicated. There is a parentQuestionType
component that has a component map which renders the respective component based on the question type set in the question object.RenderingType
- similar toQuestionType
except for rendering types (e.g. -date, number, text).question
(orformfield
) object, a context is used that is provided in theQuestionModal
and used in all the children components. A context was used because otherwise I would have prop drilled down a state.form-engine-lib
and not the ones typed within the form builder itself.2. Adjust the code structure
interactive-builder
folder.interactive-builder/modal
/tools
3. Additional changes
Added aliases to the folders hooks, resources, and tools and to the types and constants file to avoid long relative paths - these were added to the webpack config and jest config.
Screenshots
Related Issue
https://issues.openmrs.org/browse/O3-4145
https://issues.openmrs.org/browse/O3-3950
Other
Work left to do:
form-engine-lib
. Most types were re-defined within form builder, and are heavily outdated. We should change it to directly import the types from there instead.