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

Company layout cleanup #6464

Merged

Conversation

cgsunkel
Copy link
Contributor

@cgsunkel cgsunkel commented Jan 26, 2024

Description of change

This is a cleanup PR for the company layout refactoring work.

The following small changes have been made:

  • The old CompanyLayout component has been deleted and replaced by the new one.
  • The flashMessages prop has been removed as these aren't being passed down through the controllers anymore.
  • The add/remove from company list form has now been refactored to remove the need for a specific returnUrl prop to be passed down from all the company pages.
  • The CompanyLocalHeader now gets the csrfToken from the state instead of needing it to be passed down.
  • The export collection lists now use the skeleton placeholder.
  • A couple of other very minor changes.

Test instructions

Company pages should still work as normal.

Checklist

  • Has the branch been rebased to main?
  • Automated tests (Any of the following when applicable: Unit, Functional or End-to-End)
  • Manual compatibility testing (Browsers: Chrome, Firefox, Edge, Safari)

@cgsunkel cgsunkel requested a review from a team as a code owner January 26, 2024 10:52
@@ -18,7 +18,6 @@ async function handleAddRemoveCompanyToList(req, res, next) {
listsToUpdate.push(addOrRemoveCompany(req, listId, id))
}
await Promise.all(listsToUpdate)
req.flash('success', 'Lists changes for this company have been saved.')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This flash message is already being generated in the form

@cgsunkel cgsunkel force-pushed the chore/company-layout-cleanup branch from 861b15d to 69df874 Compare January 26, 2024 10:56
Copy link

cypress bot commented Jan 26, 2024

Passing run #50551 ↗︎

0 75 7 0 Flakiness 0

Details:

Fixup! use URL for company lists
Project: data-hub-frontend Commit: 63a5d65116
Status: Passed Duration: 08:02 💡
Started: Jan 26, 2024 5:11 PM Ended: Jan 26, 2024 5:19 PM

Review all test suite changes for PR #6464 ↗︎

@cgsunkel cgsunkel force-pushed the chore/company-layout-cleanup branch from 69df874 to 4ba228d Compare January 26, 2024 11:18
Copy link
Contributor

@peterhudec peterhudec 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 left a couple of comments.

@cgsunkel cgsunkel merged commit 2c6212d into feature-merge/company-layout-refactoring Jan 26, 2024
14 checks passed
@cgsunkel cgsunkel deleted the chore/company-layout-cleanup branch January 26, 2024 17:18
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