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

BUG Display a notification after publishing a campaign #177

Merged
merged 2 commits into from
Jan 19, 2021

Conversation

maxime-rainville
Copy link
Contributor

@maxime-rainville maxime-rainville commented Sep 16, 2020

@maxime-rainville
Copy link
Contributor Author

Might need to wrtie some unit test and behat test + bump dependencies.

client/lang/src/en.json Outdated Show resolved Hide resolved
client/src/state/campaign/CampaignActions.js Outdated Show resolved Hide resolved
@bergice
Copy link
Contributor

bergice commented Oct 12, 2020

@maxime-rainville, seems to work, just need to update the text and fix travis.

@maxime-rainville
Copy link
Contributor Author

Updated the messages and added some behat test. The behat test won't be green until the matching framework PR is merged.

Copy link

@sachajudd sachajudd left a comment

Choose a reason for hiding this comment

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

Approved from UX perspective.

Copy link
Member

@emteknetnz emteknetnz left a comment

Choose a reason for hiding this comment

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

Code looks fine and tests fine

It might be worth creating another issue for making a toast notification for when you inline CREATE a campaign

image

You've got a behat timeout on travis that'll need to be fixed

@maxime-rainville
Copy link
Contributor Author

Just had a glance to see if we could sneak in a "successfully added" toast notification is this PR. Sadly, it looks non-trivial.

The add to campaign logic is actually in admin for some weird reason. https://github.com/silverstripe/silverstripe-admin/blob/1/client/src/legacy/AddToCampaignForm.js

@maxime-rainville
Copy link
Contributor Author

Added a follow up issue for "the add to campaign" modal #190

@emteknetnz emteknetnz merged commit 8235fdc into silverstripe:1 Jan 19, 2021
@emteknetnz emteknetnz deleted the pulls/1/toast-react branch January 19, 2021 03:12
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.

User should receive a toast confirmation when publishing a campaign
4 participants