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

Add link/button to show/hide task instructions #8431

Merged
merged 19 commits into from
Dec 28, 2018

Conversation

rkreyhsig
Copy link
Contributor

@rkreyhsig rkreyhsig commented Dec 24, 2018

Resolves #8217 - Add link/button to show/hide task instructions

Before After

show_hide_task_instructions

@ghost ghost assigned rkreyhsig Dec 24, 2018
@ghost ghost added the In-Progress label Dec 24, 2018
@rkreyhsig rkreyhsig changed the title Initial commit Add link/button to show/hide task instructions Dec 26, 2018
name={(this.state.taskInstructionsIsVisible ? 'Hide ' : 'View ') +
COPY.TASK_SNAPSHOT_TASK_INSTRUCTIONS_LABEL}
onClick={this.toggleTaskInstructionsVisibility} />
{ this.state.taskInstructionsIsVisible &&
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's put the "Show/hide task instructions" link text below the task instructions if the instructions are visible to match the mocks.

@lowellrex
Copy link
Contributor

While we're in here messing with the TaskSnapshot component let's add a paddingBottom: '3rem' to taskContainerStyling so that the rows have a little more room to breathe.

linkStyling
styling={css({ padding: '0' })}
name={(this.state.taskInstructionsIsVisible ? 'Hide ' : 'View ') +
COPY.TASK_SNAPSHOT_TASK_INSTRUCTIONS_LABEL}
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's just make this two separate pieces of new copy in COPY.json instead of concatenating "Hide" and "View" every time.

client/COPY.json Outdated
@@ -51,6 +51,9 @@
"TASK_SNAPSHOT_TASK_TYPE_LABEL": "Task",
"TASK_SNAPSHOT_TASK_FROM_LABEL": "Assigned by",
"TASK_SNAPSHOT_TASK_INSTRUCTIONS_LABEL": "Task instructions",
"TASK_SNAPSHOT_VIEW_TASK_INSTRUCTIONS_LABEL": "View Task instructions",
"TASK_SNAPSHOT_HIDE_TASK_INSTRUCTIONS_LABEL": "Hide Task instructions",
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's lowercase "Task" to match the mocks.

@@ -257,6 +257,7 @@

click_on "Pal Smith"

find("button", text: "View " + COPY::TASK_SNAPSHOT_TASK_INSTRUCTIONS_LABEL).click
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's update the tests to use the new copy we just added.

Copy link
Contributor

@lowellrex lowellrex left a comment

Choose a reason for hiding this comment

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

LGTM!

@rkreyhsig rkreyhsig merged commit 9e9a824 into master Dec 28, 2018
@ghost ghost removed the In-Progress label Dec 28, 2018
@pkarman pkarman deleted the rkreyhsig/8217_add_show_hide_task_instructions branch January 7, 2019 22:08
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