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

Integrate manufacturer into catalogue items v2 #108 #144

Merged

Conversation

MatteoGuarnaccia5
Copy link
Contributor

@MatteoGuarnaccia5 MatteoGuarnaccia5 commented Nov 23, 2023

Description

Integrated manufacturer into catalogue items, changed dialog so that users are able to select a manufacturer from a autocomplete dropdown. they can also add a new manufacturer, which will then be added to dropdown list. The ID of the manufacturer is then passed through to the backend to be added to the catalogue item. Currently the table view displays all the fields of a manufacturer (name, address ,url, telephone number) but this is subject to change based upon CLFs feedback

Note - #132 merged into this PR. So this PR will essentially close it and the issue for it

Testing instructions

Add a set up instructions describing how the reviewer should test the code

  • Review code
  • Check Actions build
  • Review changes to test coverage

Agile board tracking

closes #108
closes #129

Base automatically changed from MUI-table-V2-#131 to develop November 27, 2023 12:01
@MatteoGuarnaccia5
Copy link
Contributor Author

I tried to simplify the logic for selectedManufacturer in the Dialog. The solution I found doesn't seem to work when you then navigate to the landing page and try to edit an item from there. I can keep trying if you guys wish but it is as compact as I can make it

@joelvdavies
Copy link
Collaborator

I tried to simplify the logic for selectedManufacturer in the Dialog. The solution I found doesn't seem to work when you then navigate to the landing page and try to edit an item from there. I can keep trying if you guys wish but it is as compact as I can make it

If this is about the comment I made in catalogueItemsDialog, replacing
const { data: selectedCatalogueItemManufacturer } = useManufacturer( selectedCatalogueItem?.manufacturer_id );
with
const selectedCatalogueItemManufacturer = manufacturerList?.find( (manufactuerer) => manufactuerer.id === selectedCatalogueItem?.manufacturer_id ) || null;
appears to work. But don't worry about it if not.

Copy link
Collaborator

@joelvdavies joelvdavies left a comment

Choose a reason for hiding this comment

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

Last two things from me
The address on the manufacturers landing page is well spaced out
image

But on the catalogue items page it is not
image

There are also still a few ListItems in catalogueItemsLandingPage.component.tsx.

Copy link
Collaborator

@joshuadkitenge joshuadkitenge left a comment

Choose a reason for hiding this comment

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

The catalogue Item landing page styling needs to be same as what it was before but using typographys

Before:
image

After:
image

This styling will also need to carry through into the manufacturer landing page so that it looks similar to the catalogue items landing page

Copy link
Collaborator

@joshuadkitenge joshuadkitenge left a comment

Choose a reason for hiding this comment

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

Also, can you change how the manufacturer address is displayed so it matches what is in the table. below:
image
This format in both of the landing pages.

As currently the address is not in the most readable format
image

@joshuadkitenge joshuadkitenge self-requested a review December 13, 2023 16:25
joshuadkitenge
joshuadkitenge previously approved these changes Dec 13, 2023
@MatteoGuarnaccia5
Copy link
Contributor Author

Needs to be merged in first priority after christmas

Copy link
Collaborator

@joshuadkitenge joshuadkitenge left a comment

Choose a reason for hiding this comment

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

LGTM

@MatteoGuarnaccia5 MatteoGuarnaccia5 merged commit d13e242 into develop Jan 2, 2024
4 checks passed
@MatteoGuarnaccia5 MatteoGuarnaccia5 deleted the integrate-manufacturer-into-catalogue-items-v2-#108 branch January 2, 2024 10:59
@MatteoGuarnaccia5 MatteoGuarnaccia5 mentioned this pull request Jan 2, 2024
6 tasks
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.

Create manufacturer landing page Intergrate manufacturer into the catalogue item dialog
3 participants