Skip to content

feat: update notification frontend#926

Merged
tom-rm-meyer-ISST merged 5 commits intoeclipse-tractusx:mainfrom
achtzig20:feat/update-notification-frontend
Aug 6, 2025
Merged

feat: update notification frontend#926
tom-rm-meyer-ISST merged 5 commits intoeclipse-tractusx:mainfrom
achtzig20:feat/update-notification-frontend

Conversation

@OlgaIvkovic
Copy link
Copy Markdown
Contributor

@OlgaIvkovic OlgaIvkovic commented Jul 31, 2025

Description

  • Update notification following standard version 3
  • Implemented forwarding, updating, resolving notifications
  • Adjusted design of the page

Resolves #918

Pre-review checks

Please ensure to do as many of the following checks as possible, before asking for committer review:

  • DEPENDENCIES are up-to-date. Dash license tool. Committers can open IP issues for restricted libs.
  • Copyright and license header are present on all affected files
  • If helm chart has been changed, the chart version has been bumped to either next major, minor or patch level (compared to released chart).

Copy link
Copy Markdown
Contributor

@tom-rm-meyer-ISST tom-rm-meyer-ISST left a comment

Choose a reason for hiding this comment

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

Thanks for your first contribution! Works very well. I only found a few things that need to be considered for now. Please check.

Comment thread frontend/src/components/ui/DateTime.tsx
Comment thread frontend/src/features/notifications/components/CollapsibleNotification.tsx Outdated
Copy link
Copy Markdown
Contributor

@tom-rm-meyer-ISST tom-rm-meyer-ISST left a comment

Choose a reason for hiding this comment

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

Thanks for the fast incorporations. Noticed one thing I missed during review. Please update and I'll merge! :)

@@ -112,56 +124,85 @@ const DemandCapacityNotificationView = ({ demandCapacityNotification, partners }
<FormLabel>Text</FormLabel>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
<FormLabel>Text</FormLabel>
<FormLabel>Text*</FormLabel>

Didn't see that last time. There should be an asteric to indicate the mandatoryness now.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sorry for missing that small detail, thanks for catching it! :)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the further consistent changes! While walking through the changes, I noticed that we should also add the asteric for the resolution modal. Please incroporate that, too :)

Copy link
Copy Markdown
Contributor

@tom-rm-meyer-ISST tom-rm-meyer-ISST left a comment

Choose a reason for hiding this comment

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

Thanks for further iterating over it! You already increased quality. Sorry for not seeing the things in the comments directly. Please check & incorporate.

@@ -112,56 +124,85 @@ const DemandCapacityNotificationView = ({ demandCapacityNotification, partners }
<FormLabel>Text</FormLabel>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the further consistent changes! While walking through the changes, I noticed that we should also add the asteric for the resolution modal. Please incroporate that, too :)

Copy link
Copy Markdown
Contributor

@tom-rm-meyer-ISST tom-rm-meyer-ISST left a comment

Choose a reason for hiding this comment

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

LGTM, thanks a lot for iterating over that with me AND your first contribution! :)

@tom-rm-meyer-ISST tom-rm-meyer-ISST merged commit 3bee99e into eclipse-tractusx:main Aug 6, 2025
13 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.

Update Notification frontend

2 participants