Issue 89: UI to edit non-ui driven xp fields #141
Conversation
djsteinmetz
commented
Jun 14, 2023
djsteinmetz
commented
- Add UI to create/edit/delete non ui related xp fields
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
I left some review comments on here, as well as on vercel. I think if you just look at the preview deployment you should be able to see them in the UI but let me know if that is not the case
Can you please additionally clean up all instances of the XpCard component (old xp builder)
| justifyContent={"center"} | ||
| minH={"xs"} | ||
| > | ||
| {Object.values(nonUiXp)?.map((xp, idx) => { |
There was a problem hiding this comment.
nitpick: can we remove ?. here, not needed
| colorScheme="primary" | ||
| onClick={() => { | ||
| setXpPropertyNameToEdit(Object.keys(nonUiXp)?.[idx]) | ||
| setXpPropertyValueToEdit(xp) |
There was a problem hiding this comment.
nitpick: can we remove ?. here, not needed
| textTransform="uppercase" | ||
| letterSpacing={1} | ||
| > | ||
| {Object.keys(nonUiXp)?.[idx]} |
There was a problem hiding this comment.
nitpick: can we remove ?. here, not needed
|
All comments have been addressed. I also created a new issue related, to use this genericize this xp component and use it for all resources. #144 |
|
@djsteinmetz @crhistianramirez - I realize it probably worked this way before, but do we think that it makes sense to track XP changes the same as other tabs? Where it only saves changes when the user hits the Save button and the "discard changes" will undo anything you changed in Xp tab? Thinking like if you delete or change a property then want to "discard" those changes. Also less saves / syncs if we save all changes at once - so in theory more performant. *EDIT I could be convinced of either way. Though, it does seem the "delete" doesn't actually remove the property. If we don't go with updating product xp on HoC Save button, I think there should be a "confirm" step to removing properties. |
