Skip to content

Remove success banner on container state change#14659

Merged
mushtaqak merged 1 commit intomushtaq/move-componentfrom
mushtaq/remove-banner-logging
Mar 13, 2017
Merged

Remove success banner on container state change#14659
mushtaqak merged 1 commit intomushtaq/move-componentfrom
mushtaq/remove-banner-logging

Conversation

@mushtaqak
Copy link
Contributor

@mushtaqak mushtaqak commented Mar 9, 2017

This PR adds the following capabilities to move feature :

  • Add logging on move xblock
  • Add i18n to backend error messages in case they appear to end user
  • Remove success banner when trying to publish or discard changes on container page

Sandbox

Copy link
Contributor Author

Choose a reason for hiding this comment

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

VisibilityState is not used in the file

@muzaffaryousaf
Copy link

@mushtaqak I'm not able to see i18n & logging in this PR.

@mushtaqak
Copy link
Contributor Author

@muzaffaryousaf @muhammad-ammar review ?

@mushtaqak
Copy link
Contributor Author

jenkins run js

Choose a reason for hiding this comment

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

Can we add coverage for this action ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will see where we can add this, may be in bokchoy.

@muzaffaryousaf
Copy link

@mushtaqak I've only 1 question otherwise looks good. 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

@mushtaqak My only concern is that some of the above messages are not user friendly. We can release the changes now but we should create a new ticket to improve the log messages.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

operation argument is missing docs

Copy link
Contributor

@muhammad-ammar muhammad-ammar left a comment

Choose a reason for hiding this comment

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

👍 after green build

@mushtaqak mushtaqak force-pushed the mushtaq/remove-banner-logging branch 2 times, most recently from 046b595 to d710262 Compare March 10, 2017 13:22
Add i18n to backend error messages in case they appear to end user
Remove success banner when trying to publish or discard changes on container page
@mushtaqak mushtaqak force-pushed the mushtaq/remove-banner-logging branch from d710262 to cb2d78b Compare March 10, 2017 14:25
@mushtaqak
Copy link
Contributor Author

jenkins run js

@mushtaqak mushtaqak merged commit 7ab699b into mushtaq/move-component Mar 13, 2017
@mushtaqak mushtaqak mentioned this pull request Mar 14, 2017
@mushtaqak mushtaqak deleted the mushtaq/remove-banner-logging branch May 10, 2017 08:02
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.

3 participants

Comments