Feature/plan/dmp overview static frontend #356

fraserclark · 2025-02-27T14:50:11Z

Description

Pulling in the remaining parts of DMP plans.

Downloads
Feedback/Access/Invite
Publish flow

I need to add translations to some of the pages these were built previously just hadn't merged them in properly; Will finish that off tomorrow.

Type of change

New feature (non-breaking change which adds functionality)

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I updated the CHANGELOG.md and added documentation if necessary
I have added tests that prove my fix is effective or that my feature works
I have completed manual or automated accessibility testing for my changes
New and existing unit tests pass locally with my changes
Any dependent changes have been merged and published in downstream modules

…tend

fraserclark · 2025-02-27T14:51:27Z

app/[locale]/projects/[projectId]/dmp/[dmpid]/download/page.tsx

I wasn't sure how translations would work with this page, so kind of left its pretty minimal. As guess a lot of the values will come from the database etc?

fraserclark · 2025-02-27T14:52:07Z

app/[locale]/projects/[projectId]/dmp/[dmpid]/page.tsx

@@ -277,6 +300,149 @@ const PlanOverviewPage: React.FC = () => {
          </div>
        </SidebarPanel>
      </LayoutWithPanel>
+
+
+      <Modal isDismissable isOpen={isPublishModalOpen}>


Modal for publish flow. I wonder whether these modals should be componentized?

I agree that eventually we should make all of these into shared components.

fraserclark · 2025-02-27T14:53:18Z

styles/form/_radio.scss

    font-weight: 400;
  }
 }

 .react-aria-Radio {
  display: flex;
-  align-items: center;
+  align-items: flex-start;


Potentially semi breaking change - but the radio button when we had helper text it put the "radio" in an awkward location, so I placed at the top again.

I think it'll be fine. It didn't break other pages with radio buttons as far as I can tell

jupiter007

Thanks Fraser. I just had a few comments. Some of these changes do need to be made to address linting issues.

jupiter007 · 2025-02-27T17:04:50Z

...ocale]/projects/[projectId]/dmp/[dmpid]/download/ProjectsProjectPlanDownloadPage.module.scss

+
+/* File format selection */
+.formatChoices {
+  display: flex;


For any space styles, like gap or margin, are we supposed to be using the shared --space-# styles?

jupiter007 · 2025-02-27T17:08:12Z

app/[locale]/projects/[projectId]/dmp/[dmpid]/feedback/__tests__/page.spec.tsx

+
+
+expect.extend(toHaveNoViolations);
+jest.mock('next-intl', () => ({


L9 - Just an FYI. If you merge in the latest from development, you will get the mock for useTranslations for free, since it was added to the 1jest.setup.ts1 file

jupiter007 · 2025-02-27T17:09:20Z

app/[locale]/projects/[projectId]/dmp/[dmpid]/feedback/invite/page.tsx

+
+
+
+      <LayoutWithPanel>


L99: I think we can use LayoutContainer here instead of LayoutWithPanel since there is no sidebar on this page.

Good spot - will fix

jupiter007 · 2025-02-27T17:09:40Z

app/[locale]/projects/[projectId]/dmp/[dmpid]/feedback/invite/page.tsx

+            </Form>
+          </div>
+        </ContentContainer>
+        <SidebarPanel/>


L192: Don't think the SidebarPanel is needed here?

jupiter007 · 2025-02-27T17:10:54Z

app/[locale]/projects/[projectId]/dmp/[dmpid]/feedback/invite/page.tsx

+              <div className={styles.formGroup}>
+                <h3 className={styles.sectionLabel}>What should this person be
+                  able to do?</h3>
+                <CheckboxGroup


L133: Just an FYI - We have a shared CheckboxGroupComponent now at components/Form/CheckboxGroup. No worries though. We can update this as we're working on hooking up the page.

Yeah sorry, I will double check these iņ the future

jupiter007 · 2025-02-27T17:12:09Z

app/[locale]/projects/[projectId]/dmp/[dmpid]/page.tsx

@@ -277,6 +300,149 @@ const PlanOverviewPage: React.FC = () => {
          </div>
        </SidebarPanel>
      </LayoutWithPanel>
+
+
+      <Modal isDismissable isOpen={isPublishModalOpen}>


I agree that eventually we should make all of these into shared components.

jupiter007 · 2025-02-27T17:13:51Z

styles/form/_radio.scss

    font-weight: 400;
  }
 }

 .react-aria-Radio {
  display: flex;
-  align-items: center;
+  align-items: flex-start;


I think it'll be fine. It didn't break other pages with radio buttons as far as I can tell

jupiter007 · 2025-02-27T17:17:31Z

app/[locale]/projects/[projectId]/dmp/[dmpid]/feedback/invite/page.tsx

+    }
+
+    // Form submission logic
+    console.log('Inviting user:', {email, accessType});


L47: Did you mean to keep the console.log here?

I will remove

jupiter007 · 2025-02-27T17:18:44Z

app/[locale]/projects/[projectId]/dmp/[dmpid]/feedback/invite/page.tsx

+
+              <div className={styles.formHelp}>
+                <p>
+                  When you click <strong>Grant access</strong> we'll send an


L176, 181, 182 - I think we need to replace all apostrophes in the page with an entity like ‘, otherwise the build will pick up on the linting error.

jupiter007 · 2025-02-27T17:20:17Z

app/[locale]/projects/[projectId]/dmp/[dmpid]/page.tsx

@@ -14,6 +23,15 @@ import {

 const PlanOverviewPage: React.FC = () => {
  const t = useTranslations('PlanOverview');
+  let [isPublishModalOpen, setPublishModalOpen] = useState(false);


L26, 27, 28 - The 'let' in all three lines can be changed to a 'const' since these do not change.

fraserclark added 4 commits February 26, 2025 16:25

Add publish and download modal functionality for DMPs

0f2ac62

Merge branch 'development' into feature/plan/dmp-overview-static-fron…

7d7c469

…tend

Merge branch 'development' into feature/plan/dmp-overview-static-fron…

1136b54

…tend

Merge the feedback page and invite modal

d7c19a1

fraserclark commented Feb 27, 2025

View reviewed changes

jupiter007 reviewed Feb 27, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/plan/dmp overview static frontend #356

Feature/plan/dmp overview static frontend #356

fraserclark commented Feb 27, 2025

fraserclark Feb 27, 2025

fraserclark Feb 27, 2025

jupiter007 Feb 27, 2025

fraserclark Feb 27, 2025

jupiter007 Feb 27, 2025

jupiter007 left a comment

jupiter007 Feb 27, 2025

jupiter007 Feb 27, 2025

jupiter007 Feb 27, 2025

fraserclark Feb 28, 2025

jupiter007 Feb 27, 2025

jupiter007 Feb 27, 2025

fraserclark Feb 28, 2025

jupiter007 Feb 27, 2025

jupiter007 Feb 27, 2025

jupiter007 Feb 27, 2025

fraserclark Feb 28, 2025

jupiter007 Feb 27, 2025

jupiter007 Feb 27, 2025



		expect.extend(toHaveNoViolations);
		jest.mock('next-intl', () => ({

Feature/plan/dmp overview static frontend #356

Are you sure you want to change the base?

Feature/plan/dmp overview static frontend #356

Conversation

fraserclark commented Feb 27, 2025

Description

Type of change

Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jupiter007 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment