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

EES-5544 Change data set import page accordions into a table #5459

Open
wants to merge 7 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,12 @@
"prettier --write"
],
"*.robot": [
"pipenv run robotidy --config tests/robot-tests/robotidy.toml"
"python3 -m pipenv run robotidy --config tests/robot-tests/robotidy.toml"
],
"*.py": [
"pipenv run flake8",
"pipenv run black",
"pipenv run isort"
"python3 -m pipenv run flake8",
"python3 -m pipenv run black",
"python3 -m pipenv run isort"
]
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ public static ErrorViewModel GenerateErrorDataReplacementAlreadyInProgress()

public static readonly LocalizableMessage DataSetTitleTooLong = new(
Code: nameof(DataSetTitleTooLong),
Message: "Subject title '{0}' must be 120 characters or less"
Message: "Title '{0}' must be 120 characters or less"
);

public static ErrorViewModel GenerateErrorDataSetTitleTooLong(string title)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ export default function ReleaseDataFilePage({
.required('Enter a title')
.max(
titleMaxLength,
`Subject title must be ${titleMaxLength} characters or less`,
`Title must be ${titleMaxLength} characters or less`,
),
})}
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,7 @@ describe('ReleaseDataFileReplacePage', () => {
);

await waitFor(() => {
expect(screen.getByTestId('Subject title')).toHaveTextContent(
'Test data',
);
expect(screen.getByTestId('Title')).toHaveTextContent('Test data');
expect(screen.getByTestId('Data file')).toHaveTextContent('data.csv');
expect(screen.getByTestId('Metadata file')).toHaveTextContent(
'data.meta.csv',
Expand Down Expand Up @@ -216,7 +214,7 @@ describe('ReleaseDataFileReplacePage', () => {

await waitFor(() => {
// Replacement
expect(screen.getByTestId('Replacement Subject title')).toHaveTextContent(
expect(screen.getByTestId('Replacement Title')).toHaveTextContent(
'Test data',
);
expect(screen.getByTestId('Replacement Data file')).toHaveTextContent(
Expand All @@ -242,17 +240,15 @@ describe('ReleaseDataFileReplacePage', () => {
);

// Original
expect(screen.getByTestId('Subject title')).toHaveTextContent(
'Test data',
);
expect(screen.getByTestId('Title')).toHaveTextContent('Test data');
expect(screen.getByTestId('Data file')).toHaveTextContent('data.csv');
expect(screen.getByTestId('Metadata file')).toHaveTextContent(
'data.meta.csv',
);
expect(screen.getByTestId('Data file size')).toHaveTextContent('200 B');
expect(screen.getByTestId('Number of rows')).toHaveTextContent('100');
expect(screen.getByTestId('Status')).toHaveTextContent(
'Data replacement in progress',
'Replacement in progress',
);
expect(screen.getByTestId('Uploaded by')).toHaveTextContent(
'original@test.com',
Expand Down Expand Up @@ -305,17 +301,15 @@ describe('ReleaseDataFileReplacePage', () => {
),
).toBeInTheDocument();

expect(screen.getByTestId('Subject title')).toHaveTextContent(
'Test data',
);
expect(screen.getByTestId('Title')).toHaveTextContent('Test data');
expect(screen.getByTestId('Data file')).toHaveTextContent('data.csv');
expect(screen.getByTestId('Metadata file')).toHaveTextContent(
'data.meta.csv',
);
expect(screen.getByTestId('Data file size')).toHaveTextContent('200 B');
expect(screen.getByTestId('Number of rows')).toHaveTextContent('100');
expect(screen.getByTestId('Status')).toHaveTextContent(
'Data replacement in progress',
'Replacement in progress',
);
expect(screen.getByTestId('Uploaded by')).toHaveTextContent(
'original@test.com',
Expand All @@ -325,9 +319,7 @@ describe('ReleaseDataFileReplacePage', () => {
);

expect(screen.getByText('Pending data replacement')).toBeInTheDocument();
expect(
screen.getByText('Data replacement in progress'),
).toBeInTheDocument();
expect(screen.getByText('Replacement in progress')).toBeInTheDocument();
expect(
screen.getByText(
'There was a problem loading the data replacement information.',
Expand Down Expand Up @@ -376,9 +368,7 @@ describe('ReleaseDataFileReplacePage', () => {

await waitFor(() => {
expect(screen.getByText('Pending data replacement')).toBeInTheDocument();
expect(
screen.getByText('Data replacement in progress'),
).toBeInTheDocument();
expect(screen.getByText('Replacement in progress')).toBeInTheDocument();

expect(screen.getByText('Data blocks: OK')).toBeInTheDocument();
expect(screen.getByText('Footnotes: OK')).toBeInTheDocument();
Expand Down Expand Up @@ -433,9 +423,7 @@ describe('ReleaseDataFileReplacePage', () => {

await waitFor(() => {
expect(screen.getByText('Pending data replacement')).toBeInTheDocument();
expect(
screen.getByText('Data replacement in progress'),
).toBeInTheDocument();
expect(screen.getByText('Replacement in progress')).toBeInTheDocument();
expect(
screen.getByText(/The replacement data file is still being processed/),
);
Expand Down Expand Up @@ -485,9 +473,7 @@ describe('ReleaseDataFileReplacePage', () => {

await waitFor(() => {
expect(screen.getByText('Pending data replacement')).toBeInTheDocument();
expect(
screen.getByText('Data replacement in progress'),
).toBeInTheDocument();
expect(screen.getByText('Replacement in progress')).toBeInTheDocument();
expect(
screen.getByText(
'There was a problem loading the data replacement information.',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,11 @@ const DataFileDetailsTable = ({
<tbody>
<tr>
<th scope="row" className="govuk-!-width-one-third">
Subject title
Title
</th>
<td data-testid="Subject title">{dataFile.title}</td>
<td data-testid="Title">{dataFile.title}</td>
{replacementDataFile && (
<td data-testid="Replacement Subject title">
{replacementDataFile.title}
</td>
<td data-testid="Replacement Title">{replacementDataFile.title}</td>
)}
</tr>
<tr>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ const subjectErrorMappings = [
mapFieldErrors<DataFileUploadFormValues>({
target: 'subjectTitle',
messages: {
SubjectTitleMustBeUnique: 'Subject title must be unique',
SubjectTitleMustBeUnique: 'Title must be unique',
SubjectTitleCannotContainSpecialCharacters:
'Subject title cannot contain special characters',
'Title cannot contain special characters',
},
}),
];
Expand Down Expand Up @@ -200,7 +200,7 @@ export default function DataFileUploadForm({
})
.max(
titleMaxLength,
`Subject title must be ${titleMaxLength} characters or less`,
`Title must be ${titleMaxLength} characters or less`,
),
}),
});
Expand Down Expand Up @@ -239,7 +239,7 @@ export default function DataFileUploadForm({
{!isDataReplacement && uploadType !== 'bulkZip' && (
<FormFieldTextInput<DataFileUploadFormValues>
name="subjectTitle"
label="Subject title"
label="Title"
className="govuk-!-width-two-thirds"
maxLength={titleMaxLength}
/>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
@import '~govuk-frontend/dist/govuk/base';

.table {
td {
vertical-align: middle;
}
}

.title {
max-width: 500px;
}

.fileSize {
width: 5rem;
text-align: right;
}

.fileStatus {
max-width: 126px;
}

.actions {
margin-bottom: 0;

@include govuk-media-query($until: desktop) {
align-items: start;
flex-direction: column;

a,
button {
margin-bottom: 1rem;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems like the spacing is a bit excessive now:

image

Additionally, we should be using govuk-spacing function for this.

1 spacing unit:

image

2 spacing units:

image

Feels like 1 spacing unit might actually be fine

margin-left: 0;
white-space: nowrap;
}
}
}
Loading
Loading