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

redesign landing page systems #1024 #1025

Open
wants to merge 19 commits into
base: develop
Choose a base branch
from

Conversation

joshuadkitenge
Copy link
Collaborator

@joshuadkitenge joshuadkitenge commented Oct 11, 2024

Description

  • Redesign for System landing page #1024
  • Add icon for items
  • tabs for systems page
  • you actions dropdown for when upload and image and attachments is added
  • Edit the fontsizes. The title front is the biggest now
  • improve table view

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
  • {more steps here}

Agile board tracking

closes #1024

@joshuadkitenge joshuadkitenge added the enhancement Improved or refactored feature label Oct 11, 2024
Copy link

codecov bot commented Oct 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.15%. Comparing base (d7fe419) to head (d4c614a).
Report is 17 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1025      +/-   ##
===========================================
+ Coverage    99.13%   99.15%   +0.01%     
===========================================
  Files           78       77       -1     
  Lines        16385    16406      +21     
  Branches      2785     2012     -773     
===========================================
+ Hits         16244    16268      +24     
  Misses         137      137              
+ Partials         4        1       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@joshuadkitenge joshuadkitenge force-pushed the redesign-landing-page-catalogue-items-#1022 branch from cbac313 to 40ad999 Compare October 17, 2024 08:50
@joshuadkitenge joshuadkitenge marked this pull request as ready for review October 17, 2024 13:53
@joelvdavies
Copy link
Collaborator

joelvdavies commented Oct 17, 2024

Just a general thought - I think it would be worth pushing instead of replacing the URL? Given how the table states can go forwards and back and now that the default is removed from the URL I think it should work without being a nuisance.

Also just noticed there are two sets of tabPanel.component files, one in a tab folder and one a level above 😄

@joshuadkitenge joshuadkitenge force-pushed the redesign-landing-page-systems-#1024 branch from 2a1ddd2 to 7dd2969 Compare October 17, 2024 15:56
src/common/tab/tabView.component.tsx Outdated Show resolved Hide resolved
src/common/tab/tabView.component.tsx Show resolved Hide resolved
src/systems/systemDetails.component.test.tsx Outdated Show resolved Hide resolved
src/systems/systemDetails.component.tsx Outdated Show resolved Hide resolved
Base automatically changed from redesign-landing-page-catalogue-items-#1022 to develop October 18, 2024 15:15
Comment on lines +190 to +194
<TabView
ariaLabelPrefix="systems page"
defaultTab="Items"
galleryEntityId={system.id}
attachmentsEntityId={system.id}
Copy link
Contributor

Choose a reason for hiding this comment

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

Does galleryEntityID and attachmentsEntityId refer to the id of the actual "gallery/attachment object" or their parent/foreign key (in this case system.id).

If its the former, I assume system.id is just a placeholder until they get implemented, but if its the latter it would make sense to combine parameters.

@@ -7,64 +7,93 @@ import { useSearchParams } from 'react-router-dom';
import { a11yProps, StyledTab } from '../../common/tab/tab.utils';
import TabPanel from '../../common/tab/tabPanel.component';

type AdditionalTabValues = 'Gallery' | 'Attachments';

export interface TabViewProps<T> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
export interface TabViewProps<T> {
export interface TabViewProps<T extends string> {

should mean that you can remove all of the as unknown as string castings. Similar change could be made to a11yProps

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improved or refactored feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Redesign for System landing page
3 participants