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

Implement Manage Groupings #102

Merged
merged 1 commit into from
Nov 2, 2024
Merged

Implement Manage Groupings #102

merged 1 commit into from
Nov 2, 2024

Conversation

hokwaichan
Copy link
Contributor

@hokwaichan hokwaichan commented Oct 17, 2024

Ticket Link

1728

List of squashed commits

  • Implement Manage Groupings

Test Checklist

  • Unit Tests Passed
  • Integration Tests Passed
  • General Visual Inspection

@hokwaichan hokwaichan requested a review from JorWo October 17, 2024 09:14
@JorWo
Copy link
Contributor

JorWo commented Oct 17, 2024

Could you add the styling for the All Members title and include the arrow like so?:
image

@JorWo
Copy link
Contributor

JorWo commented Oct 17, 2024

The color of these icons should be #004252 and the circle should be slightly smaller.
image

@JorWo
Copy link
Contributor

JorWo commented Oct 17, 2024

In path for /groupings/[selectedGrouping], let's rename [selectedGrouping] to [groupingPath] so that it is more clear.

@JorWo
Copy link
Contributor

JorWo commented Oct 17, 2024

Could you add the rest of the routes (basis, include, exclude, owners, sync-destinations, preferences)? Thanks.

@JorWo
Copy link
Contributor

JorWo commented Oct 17, 2024

Add this to next-config.mjs:

rewrites: async () => {
    return [
        {
            source: '/groupings/:groupingPath',
            destination: '/groupings/:groupingPath/all-members'
        }
    ];
}

This will map /groupings/tmp:jordanw4:jordanw4-aux to the same page as /groupings/tmp:jordanw4:jordanw4-aux/all-members. Helpful to avoid a 404 error.

@hokwaichan hokwaichan force-pushed the dev-hokwai-1728 branch 6 times, most recently from f2ef599 to 44bcb74 Compare October 31, 2024 11:15
@@ -0,0 +1,27 @@
import React from 'react';
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be removed.

@@ -0,0 +1,19 @@
import React from 'react';
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be removed.

import { render, screen } from '@testing-library/react';
import SideNav from '@/app/groupings/[groupingPath]/_components/side-nav';
import { usePathname } from 'next/navigation';
import '@testing-library/jest-dom';
Copy link
Contributor

@JorWo JorWo Nov 1, 2024

Choose a reason for hiding this comment

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

import '@testing-library/jest-dom' should not be needed.

@@ -0,0 +1,78 @@
import {render, screen } from '@testing-library/react';
import '@testing-library/jest-dom';
Copy link
Contributor

@JorWo JorWo Nov 1, 2024

Choose a reason for hiding this comment

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

import '@testing-library/jest-dom' should not be needed. Also run Prettier formatter in here.

groupPath: 'Test-path:Test-name',
groupDescription: 'Test Description'
},
{}
Copy link
Contributor

Choose a reason for hiding this comment

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

This empty object might have been left here on accident.

@JorWo JorWo merged commit 0dde260 into main Nov 2, 2024
6 checks passed
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