Conversation
|
I'm going to check out React Semantic docs to see if it supports the new 16.3 Context in the Popup component as I'm not a fan in how I'm passing the |
|
It doesn't look like the code for React 16.3 has been released yet. I'll keep an 👁 on semantic and update once React 16.3 changes are released. |
mikelax
left a comment
mikelax
left a comment
There was a problem hiding this comment.
left one small comment.
Also, where is the logic that looks at the type of game to decide what fields should be included in the quick edit modal?
| import serviceExecutor from 'utils/serviceExecutor'; | ||
|
|
||
|
|
||
| export default function ({ |
There was a problem hiding this comment.
@nazar should this be a named function? is there an eslint rule for this?
There was a problem hiding this comment.
A function name is optional when it is exported as the default function.
| .chain() | ||
| .map((change) => { | ||
| return (` | ||
| <div class="change-meta"> |
There was a problem hiding this comment.
@nazar should this be a p instead of div to match the messages generated by the editor?
Also, when I look at the html that was saved to the game_messages table, I don't see the class attribute, is it possible that is getting stripped somewhere?
| .map((change) => { | ||
| return (` | ||
| <div class="change-meta"> | ||
| <strong>${changeDescription || 'Changed'}</strong>: |
There was a problem hiding this comment.
@nazar should the value here changeDescription be stored in the character_logs table in the change_description field? I think it should, right?
Eventually we are going to allow viewing the "history", or changelog of a Character, and it would be nice to see the the human entered value for the description rather than the generic Character stats updated.
There was a problem hiding this comment.
should the value here changeDescription be stored in the character_logs table in the change_description field? I think it should, right?
changeDescription should be stored character_logs - I'll double check 👍
| function doUpdateCharacter() { | ||
| const cleanInput = _(input) | ||
| .chain() | ||
| .omit(['changeDescription', 'changeMeta']) |
There was a problem hiding this comment.
@nazar should changeDescription be removed from this line here so the user entered description will override the default Character stats updated message?
Update: I tried removing the field but then the save function doesn't work, so I must be missing the reason for it.
…sages. This extra type is used to hide the "edit" button on character update game messages
| "source-map-support": "0.5.4", | ||
| "sql-fixtures": "1.0.0", | ||
| "subscriptions-transport-ws": "0.9.7", | ||
| "tsml": "^1.0.1", |
There was a problem hiding this comment.
@nazar I'll merge this PR, but do you think this lib is necessary? I saw its not maintained anymore, though I assume the function it performs is really straight forward so no need for updates??
There was a problem hiding this comment.
The lib is nice in that it strips whitespace and newlines added in backtick quoted multiline strings.
The code is < 10 lines - I did attempt to copypasta it but eslint disliked the non fancy es6 formats and I was too short on time to es5 -> es6 it.
2 participants
References #163