Conversation
lifts up the name and description values passes to header and changelogs to use in history update
|
@cpresler can you look at this update and let me know if you think lifting the name and description up a level will actually break older widgets that upgrade? |
also updated typo for depreciated
cpresler
left a comment
cpresler
left a comment
There was a problem hiding this comment.
Over all this looks good, just a few adjustments and it should be 👍
| } | ||
|
|
||
| const nameFormatted = nameParts.join(''); | ||
| const descriptionFormatted = `${description}\n\n- - - -\n\n${formattedLinks}`; |
There was a problem hiding this comment.
I think \n\n- - - -\n\n should be included in formattedLinks, so if there are no formattedLinks the - - - doesn't show up.
| version?: string, | ||
| ) { | ||
| const formattedLinks = links?.map(link => `${link.label}\n${link.url}`).join('\n\n') || ''; | ||
| if (!description.trim() && !formattedLinks.trim()) { |
There was a problem hiding this comment.
To be able to add formatting around the links as well, maybe check here against links.length === 0 instead?
Also, why does the lack of description or links stop a version history point from being created? Shouldn't a version history point always be created with a new log, even if it doesn't include a description?
| const changeLogs = useSyncedMap<ChangeLog>('changes'); | ||
| // Name and Description | ||
| const [nameText, setNameText] = useSyncedState('nameText', ''); | ||
| const [descriptionText, setDescriptionText] = useSyncedState('descriptionText', ''); |
There was a problem hiding this comment.
I tested this with some older widgets and it seems fine.
| break; | ||
| case 'added': // catch legacy logs with "added" as default type | ||
| typeFormatted = ''; | ||
| break; |
There was a problem hiding this comment.
You don't need to set this to an empty string, it already is one, you can stack the 'none' and 'added' cases together and just break.
Also, I wish we didn't need to repeat this logic since we are using something super similar in the Type component, but not sure if that is worth the refactor at this moment.
| showVersion, | ||
| version, | ||
| ) | ||
| .then(val => console.log('Version History Saved', val)) |
There was a problem hiding this comment.
nit: val is showing as undefined in the console every time so it probably isn't worth passing through.
2 participants
Description
Adds a save to history function when saving FigLogs.
Fixes # (issue)
Type of Change
How Has This Been Tested?
Checklist: