-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Show success notification once a Goal has been created or updated #23809
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
base: 5.x-dev
Are you sure you want to change the base?
Conversation
…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').
… will stop click events on the element
…lick event to 'markCurrentSegment'
…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
…props if not found in goals list
fixing test
nathangavin
left a comment
There was a problem hiding this 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.
|
There is 2 diff from the UI screenshot that seems to be legit: |
…cations after we have reloaded the page
| }); | ||
| 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>`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| 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.
| this.storeNotification(idToUse, isCreate); | ||
| this.showNotificationMessage(idToUse, isCreate); |
There was a problem hiding this comment.
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?
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
Review