Skip to content

Fixes #27005 - Fix Editor changing locked template#6840

Merged
ohadlevy merged 1 commit intotheforeman:developfrom
glekner:fix-editor-readonly
Jun 13, 2019
Merged

Fixes #27005 - Fix Editor changing locked template#6840
ohadlevy merged 1 commit intotheforeman:developfrom
glekner:fix-editor-readonly

Conversation

@glekner
Copy link
Copy Markdown
Contributor

@glekner glekner commented Jun 11, 2019

No description provided.

@theforeman-bot
Copy link
Copy Markdown
Member

Issues: #27005

Copy link
Copy Markdown
Member

@ezr-ondrej ezr-ondrej left a comment

Choose a reason for hiding this comment

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

👍 Thanks @glekner, fixes the issue and LGTM.
Only one nitpick.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

isn't the readOnly prop useless now as it is always false?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

its a textarea without onChange handler. due to the Editor taking care of the onChange, it needs to be readOnly aswell.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ack! thanks for explanation! ❤️

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

does this warrant adding a test?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

added a test. @ohadlevy

ezr-ondrej
ezr-ondrej previously approved these changes Jun 11, 2019
@ezr-ondrej
Copy link
Copy Markdown
Member

The errors are unrelated, but maybe would be better to wait for them to be resolved anyway. The fix is in #6833

amirfefer
amirfefer previously approved these changes Jun 12, 2019
Copy link
Copy Markdown
Member

@amirfefer amirfefer left a comment

Choose a reason for hiding this comment

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

Tested and works well 👍
Thanks @glekner

ohadlevy
ohadlevy previously approved these changes Jun 13, 2019
@glekner glekner dismissed stale reviews from ohadlevy and amirfefer via 161f6c5 June 13, 2019 08:55
@glekner glekner force-pushed the fix-editor-readonly branch from fce9fbc to 161f6c5 Compare June 13, 2019 08:55
@ohadlevy ohadlevy merged commit 31c0fb2 into theforeman:develop Jun 13, 2019
@glekner glekner deleted the fix-editor-readonly branch June 13, 2019 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants