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

commit label updates #3046 #3048

Merged
merged 4 commits into from
Dec 6, 2023
Merged

commit label updates #3046 #3048

merged 4 commits into from
Dec 6, 2023

Conversation

salomon-j
Copy link
Contributor

No description provided.

@salomon-j salomon-j requested a review from chrisala December 3, 2023 22:14
@@ -9,10 +9,10 @@
<tbody data-bind="foreach: rows">
<tr class="header">
<th class="code"></th>
<th class="outcome required">Outcome statement/s</th>
<th class="outcome required">${outcomeStatementHeading ?: 'Outcome Statement/s'}</th>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like most of the other default headings are sentence case - should probably use sentence case here as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ready for retesting

@@ -18,7 +18,7 @@
</g:else>
<th class="outcome required">${subtitle ?: ""} <g:if test="${helpText}"><fc:iconHelp html="true" container="body">${helpText}</fc:iconHelp></g:if> </th>
<g:if test="${extendedOutcomes}">
<th class="investment-priority required">Investment Priority</th>
<th class="investment-priority required">${investmentPriorityHeading ?: 'Investment Priority'}</th>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use sentence case for default

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ready for retesting

Copy link
Collaborator

@chrisala chrisala left a comment

Choose a reason for hiding this comment

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

Please also look at and correct the "Read only MERI Plan" template as well as the Comparison template

@salomon-j
Copy link
Contributor Author

Please also look at and correct the "Read only MERI Plan" template as well as the Comparison template

#3041 is also ready for testing, updates were pushed using the same feature branch

Copy link
Collaborator

@chrisala chrisala left a comment

Choose a reason for hiding this comment

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

Many of the view changes made hardcode labels that match NHT but will then make RLP labels wrong. The view files need to be updated to be templated to use the same variables as the editable version of the MERI plan to keep them in sync.

@@ -4,7 +4,7 @@
<table class="table">
<thead>
<tr>
<th>Project methodology</th>
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be templated using the same variable/s as meriPlan/_projectMethodology.gsp - otherwise this will just cause RLP labels to be wrong.

@@ -1,7 +1,7 @@
<table class="table">
<thead>
<tr>
<th>Project Review, Evaluation and Improvement Methodology and Approach</th>
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be templated to match meriPlan/_projectReview.gsp

@@ -11,9 +11,14 @@
<g:else>
<th class="code"></th>
</g:else>
<th class="outcome">Outcome statement/s</th>
<g:if test="${outcomeType == 'mid'}">
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be templated as per meriPlan/_outcomeStatements.gsp

@@ -5,13 +5,13 @@
<thead>
<tr>
<th class="index"></th>
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be templated as per /meriPlan/_monitoringIndicators.gsp

@@ -4,7 +4,7 @@
<table class="table">
<thead>
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be templated as per /meriPlan/_projectMethodology.gsp

@@ -2,7 +2,7 @@
<table class="table secondary-outcome">
<thead>
<tr>
<th class="outcome-priority">Secondary outcome(s)</th>
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be templated as per /meriPlan/_addionalOutcomes.gsp

@chrisala chrisala merged commit 161805f into dev Dec 6, 2023
1 check passed
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.

2 participants