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: add title and subtitle to data sets if set in display options #407

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

flaminic
Copy link
Contributor

@flaminic flaminic commented Oct 14, 2024

Implements DHIS2-18129.
This PR allows users to add a title/ substitle to datasets

Implementation details

The displayOptions was added as a JSON type in the BE to allow us flexibility to add more options without updating the back-end. See discussion in dhis2/dhis2-core#16562

Background

We are aiming to add more form configuration options as part of an initiative to provide configurations natively to data entry forms to reduce the necessity for custom forms. Users are currently building custom forms as a workaround for shortcomings of the configuration options (ability to transpose, or customise a cell design) or implementation (such to avoid issues with RTL issues).

This is an RFC that describes the approach and the priorities for form configuration options. This is based on a thorough investigation by the functional design team for custom form use cases in real-life implementations. Based on that investigation, the ability to add custom text to sections were one of the main reasons people choose to go the custom forms route so we're tackling these first.

Preview

Screenshot 2024-10-14 at 14 09 01 Screenshot 2024-10-14 at 14 08 43 Screenshot 2024-10-14 at 14 06 20

@flaminic flaminic marked this pull request as draft October 14, 2024 13:35
@dhis2-bot
Copy link
Contributor

dhis2-bot commented Oct 14, 2024

🚀 Deployed on https://pr-407--dhis2-data-entry.netlify.app

@dhis2-bot dhis2-bot temporarily deployed to netlify October 14, 2024 13:37 Inactive
@flaminic flaminic marked this pull request as ready for review October 15, 2024 06:44
@tomzemp tomzemp self-requested a review October 15, 2024 07:49
Copy link
Member

@tomzemp tomzemp 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 😊. I've left a couple of small comments.

.sectionTab {
margin-bottom: 8px;
}
.textLeft{
text-align: left;
Copy link
Member

Choose a reason for hiding this comment

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

suggest text-align: inset-inline-start to be RTL-friendly

text-align: center;
}
.textRight{
text-align: right;
Copy link
Member

Choose a reason for hiding this comment

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

suggest text-align: inset-inline-end to be RTL-friendly

margin: 2px;
font-size: 1.5em;
}
.sectionsCustomerText{
Copy link
Member

Choose a reason for hiding this comment

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

sectionsCustomText?

[styles.textLeft]:
displayOptions.customText?.align === 'left',
[styles.textCenter]:
displayOptions.customText?.align === 'center',
Copy link
Member

Choose a reason for hiding this comment

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

styles.textCenter be the default if displayOptions.customText?.align is undefined? It seems like that is the default option in the maintenance app through the UI, so I don't know if you want to set it to be the default in the data entry app as well (could occur if someone edits the display options outside of the UI)

className={styles.sectionDescription}
dangerouslySetInnerHTML={{ __html: html }}
></div>
<p className={className} dangerouslySetInnerHTML={{ __html: html }}></p>
Copy link
Member

Choose a reason for hiding this comment

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

is it intentional to change to

element? It seems that will insert an extra blank line that we didn't have before?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep this one is intentional :)

@dhis2-bot dhis2-bot temporarily deployed to netlify October 17, 2024 06:58 Inactive
@flaminic flaminic force-pushed the DHIS2-18129/add-custom-text-to-data-sets branch from fce6ee5 to 5d1f679 Compare October 17, 2024 08:10
@dhis2-bot dhis2-bot temporarily deployed to netlify October 17, 2024 08:11 Inactive
@flaminic flaminic force-pushed the DHIS2-18129/add-custom-text-to-data-sets branch from 5d1f679 to ed91642 Compare October 17, 2024 08:14
@dhis2-bot dhis2-bot temporarily deployed to netlify October 17, 2024 08:16 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants