-
Notifications
You must be signed in to change notification settings - Fork 198
(828) Show changenotes in review and show screens #9851
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
(828) Show changenotes in review and show screens #9851
Conversation
656cae0
to
da6a65f
Compare
I’ve also tightened up the selectors on the component test
We only show the change note if the change is a major one.
Again, if the change is minor, we don’t show the change note
I had the document with the copy changes in, so might as well do this now :)
da6a65f
to
42c6917
Compare
end | ||
|
||
def change_note_items | ||
content_block_edition.major_change ? [major_change_item, external_change_note_item] : [major_change_item] |
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.
don't think it really makes a difference at the moment, but an internal change note can't be added unless it's a major change right? If there is value under the internal change not field but it's not a major change, shouldn't we still show the note anyway? just in case those user-facing rules changed?
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.
Internal notes can be added regardless of if it's a major or minor. It's only public change notes that depend on major/minor
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.
Internal notes can be added regardless of if it's a major or minor.
sorry I meant external note, not internal - it's only possible to add one if it's a major change at the moment
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.
If there is value under the internal change not field but it's not a major change, shouldn't we still show the note anyway? just in case those user-facing rules changed?
was trying to say that it doesn't seem quite right to check the major_change
field instead of just checking if there's an external note, but don't think it needs to hold up the PR!
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.
Ah, gotcha!
Trello card: https://trello.com/c/eqAcYy1U/828-add-internal-and-external-change-notes-when-editing-blocks
This adds Change note related content to the review and show screens for a content block:
Screenshots