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.

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

feat: DHIS2-9652 with save buttons #1260

Merged
merged 53 commits into from
Jan 5, 2021

Conversation

paschalidi
Copy link
Contributor

@paschalidi paschalidi commented Dec 3, 2020

Hey here is the https://jira.dhis2.org/browse/DHIS2-9652👋 Again another not so small PR. Apologies I could see the best way to split this up. Will start with smaller chunks from the next one :)

What am I changing

What happens here is that we are reusing the components we created from the last two PRs and adding the buttons on the bottom. You can say it is this PR that the refactoring from the previous two PR comes together.

Now we are using the EnrollmentRegistrationEntry and TeiRegistrationEntry across the new registration page and the add relationship part of the application.

How it looks

image

@paschalidi paschalidi changed the title feat: with buttons [part 3] feat: with save buttons Dec 3, 2020
@dhis2 dhis2 deleted a comment from lgtm-com bot Dec 4, 2020
Comment on lines +38 to +48
<EnrollmentRegistrationEntry
id={dataEntryId}
selectedScopeId={selectedScopeId}
enrollmentMetadata={registrationMetaData}
saveButtonText={'Save new'}
onSave={() => alert('onSave will save in the future')}
onGetUnsavedAttributeValues={() => console.log('onGetUnsavedAttributeValues will be here in the future in the future')}
onPostProcessErrorMessage={() => console.log('onPostProcessErrorMessage will be here in the future in the future')}
onUpdateField={() => console.log('onUpdateField will be here in the future in the future')}
onStartAsyncUpdateField={() => console.log('onStartAsyncUpdateField will be here in the future in the future')}
/>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here I would say you will see the benefit of creating the EnrollmentRegistrationEntry and TeiRegistrationEntry components.

@paschalidi paschalidi requested a review from JoakimSM December 4, 2020 11:58
@paschalidi paschalidi marked this pull request as ready for review December 4, 2020 11:58
@paschalidi paschalidi changed the base branch from cp/registration/11-shows-enrollment-section to master December 7, 2020 09:48
@paschalidi paschalidi marked this pull request as draft December 7, 2020 09:50
@lgtm-com
Copy link

lgtm-com bot commented Dec 7, 2020

This pull request fixes 1 alert when merging 331529b into 4f01752 - view on LGTM.com

fixed alerts:

  • 1 for Unused variable, import, function or class

@paschalidi paschalidi marked this pull request as ready for review December 7, 2020 15:32
@paschalidi paschalidi changed the title [part 3] feat: with save buttons feat: DHIS2-9652 with save buttons Dec 7, 2020
@paschalidi paschalidi changed the title feat: DHIS2-9652 with save buttons feat: [part 01] DHIS2-9652 with save buttons Dec 10, 2020
@paschalidi paschalidi changed the title feat: [part 01] DHIS2-9652 with save buttons [part 01] feat: DHIS2-9652 with save buttons Dec 10, 2020
Copy link
Member

@JoakimSM JoakimSM left a comment

Choose a reason for hiding this comment

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

Looks good @paschalidi 👍 please have a look at my comments before you merge.

saveButtonText,
classes,
onSave,
...rest
Copy link
Member

Choose a reason for hiding this comment

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

suggestion: passOnProps

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right. I will make a PR in the end renaming all the rest to onProps.

Comment on lines +50 to +58
onSave &&
<Button
dataTest="dhis2-capture-create-and-link-button"
primary
onClick={onSave}
className={classes.marginTop}
>
{saveButtonText}
</Button>
Copy link
Member

Choose a reason for hiding this comment

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

Yep, this is fine 👍

Comment on lines +6 to +15
id: string,
enrollmentMetadata: RegistrationFormMetadata,
selectedScopeId: string,
id: string
saveButtonText: string,
fieldOptions?: Object,
onSave: (dataEntryId: string, itemId: string, formFoundation: RenderFoundation) => void,
onPostProcessErrorMessage: Function,
onGetUnsavedAttributeValues: Function,
onUpdateField: Function,
onStartAsyncUpdateField: Function,
Copy link
Member

Choose a reason for hiding this comment

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

Yep, makes sense 👍 I did the same for the working lists. Let's settle on name though. I think I called it InterfaceProps. If you don't like it, what about ModuleProps or ConceptProps? I'm not sure I like OwnProps, but if you have some compelling arguments I might reconsider. Feel free to suggest alternatives.

classes,
onPostProcessErrorMessage,
onGetUnsavedAttributeValues,
...rest
Copy link
Member

Choose a reason for hiding this comment

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

suggestion: passOnProps

<InfoOutlinedIconWithStyles />
</Grid>
<Grid item className={classes.text}>
{text}
Copy link
Member

Choose a reason for hiding this comment

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

maybe we could use children instead? Usage: <InfoIconText>text here</InfoIconText>

Comment on lines 12 to 25

const translatedTextWithStylesForProgram = (trackedEntityName, programName, orgUnitName) =>
(<>
{i18n.t('Saving a {{trackedEntityName}} in', { trackedEntityName })} <b>{programName}</b>
{orgUnitName && <>{' '}{i18n.t('in')} <b>{orgUnitName}</b></>}.
</>);


const translatedTextWithStylesForTei = (trackedEntityName, orgUnitName) =>
(<>
{i18n.t('Saving a {{trackedEntityName}}', { trackedEntityName })} <b>{i18n.t('without')}</b> {i18n.t('enrollment')}
{orgUnitName && <>{' '}{i18n.t('in')} <b>{orgUnitName}</b></>}.{' '}
{i18n.t('Enroll in a program by selecting a program from the top bar.')}
</>);
Copy link
Member

Choose a reason for hiding this comment

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

I know I've used these bold parts in the new single event page, but I've later realised the concatenations will cause problems, at least for rtl languages. I suggest we keep it nonbold for now? @Bekkalizer , any insights?

* feat: adds the buttons

* refactor: tei registration data entry

* refactor: enrollment regisration data entry

* refactor: using the components

* chore: moves the info text on the component level

* chore: lint

* chore: rest needs to be here

* chore: removes unused flow

* chore: creates TEI

* chore: saves

* Merge remote-tracking branch 'origin/master' into cp/registration/13-DHIS2-9653-submiting

* chore: reverts

* chore: adds rollback action
@paschalidi paschalidi changed the title [part 01] feat: DHIS2-9652 with save buttons feat: DHIS2-9652 with save buttons Jan 5, 2021
@paschalidi paschalidi merged commit 24ec241 into master Jan 5, 2021
@paschalidi paschalidi deleted the cp/registration/12-DHIS2-9652-withbuttons branch January 5, 2021 11:38
dhis2-bot added a commit that referenced this pull request Jan 5, 2021
# [1.4.0](v1.3.9...v1.4.0) (2021-01-05)

### Features

* DHIS2-9652 with save buttons ([#1260](#1260)) ([24ec241](24ec241)), closes [#1266](#1266)
@dhis2-bot
Copy link
Contributor

🎉 This PR is included in version 1.4.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants