Skip to content

Conversation

@chippison
Copy link
Contributor

Description

UX-333
Currently, when a Goal is edited and saved, no feedback is shown to the user.

After a Goal is successfully updated, display a success message at the top of the page.

Acceptance criteria:

Show a confirmation message like:

“Goal successfully updated. [View Goal Report]”

The “View Goal Report” link should direct to the corresponding Goal report page.

Checklist

  • [NA] I have understood, reviewed, and tested all AI outputs before use
  • [NA] All AI instructions respect security, IP, and privacy rules

Review

…f adding the segment into the store to finish before we 'mark' icons as 'compared'
…of 'data_comparison_segment_limit' to include the count for the first segment to compare (Usually 'All Visits').
…se this does not allow us to show the notifications.

We now just load the new goals by calling via api and updating the view
…. This makes it faster as well as being able to show the notification
@chippison chippison marked this pull request as ready for review November 23, 2025 21:52
@chippison chippison requested a review from a team November 23, 2025 21:52
@chippison chippison requested a review from tzi November 24, 2025 22:26
nathangavin
nathangavin previously approved these changes Nov 25, 2025
Copy link
Contributor

@nathangavin nathangavin left a comment

Choose a reason for hiding this comment

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

Looks good to me, nice cleanup on the refresh of the goals list on goal creation.

@tzi
Copy link
Contributor

tzi commented Nov 25, 2025

There is 2 diff from the UI screenshot that seems to be legit:

@chippison chippison requested a review from sgiehl November 28, 2025 03:15
@chippison chippison requested a review from mneudert December 2, 2025 01:52
});
let successMessage = translate(isCreate ? 'Goals_GoalCreated' : 'Goals_GoalUpdated');
const reportLink = `<a href="?${link}#${hash}">[${translate('Goals_ViewGoalReport')}]</a>`;
successMessage = `<div class="notification-message">${successMessage} ${reportLink}</div>`;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
successMessage = `<div class="notification-message">${successMessage} ${reportLink}</div>`;
successMessage = `${successMessage} ${reportLink}`;

In the UI test you can access the message using .notification-body, if that was the sole reason for this additional div.

Comment on lines +667 to +668
this.storeNotification(idToUse, isCreate);
this.showNotificationMessage(idToUse, isCreate);
Copy link
Member

Choose a reason for hiding this comment

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

Is flashing the notification and then doing a full page reload the wanted behaviour? Or should it only be stored here and then shown on the target page?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

6 participants