Create new entity modal and restyle update modal to match#1480
Create new entity modal and restyle update modal to match#1480
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
ktuite
left a comment
There was a problem hiding this comment.
Interactive review with @matthew-white
6cd315c to
ffbbf83
Compare
src/components/entity/upsert.vue
Outdated
| <style lang="scss"> | ||
| @import '../../assets/scss/variables'; | ||
|
|
||
| #entity-update, #entity-create { |
There was a problem hiding this comment.
@matthew-white I think you mentioned trying to change this to a .entity-upsert class instead of these IDs. I tried that and the bootstrap table tr borders came back, despite being hidden in the other CSS here. Something about CSS selector specificity. I'm thinking of leaving it like this with the IDS.
There was a problem hiding this comment.
Yeah, some of the Bootstrap table styles can be kind of overly specific. It shouldn't be too hard to match their specificity though. You can use the element inspector to find the original style, then use the same selector. Instead of tr td, th, it probably needs to be something like tbody tr td, thead tr th. I wouldn't spend more than a couple of minutes on it though. It seems a little funny for the child component to reference the parent component, but it's not a big deal.
matthew-white
left a comment
There was a problem hiding this comment.
@ktuite and I discussed this PR interactively, then I took a closer look async. I took a look at the code in src/, and it's looking great to me. There are still some open comments above that would be great to address, but nothing big. I had wanted to take a closer look at the code in test/ as well, but I haven't found time yet to do so. I'm going to go ahead and approve, since we did discuss tests interactively.
Closes getodk/central#1638
Alternate approach to: #1477
When updating the visual design, the update and create modals actually became quite similar. In the other PR, they are different enough (with the 3rd column showing the old value in the update case) that I created an entirely new create modal.
In this case, there's a shared "upsert" modal that create and update use. The row has been modified to not have a column for the old value. It has also been modified to have the label column be optional so it can be used for the label field at the top of the modal outside the table.
Create modal:

Update modal:

What has been done to verify that this works as intended?
Trying it out
Why is this the best possible solution? Were any other approaches considered?
Other approaches were considered (reusing old update modal, different styles on input fields)
How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?
Let's users make new entities in central, which will be really useful in certain cases.
Does this change require updates to user documentation? If so, please file an issue here and include the link below.
no.
Before submitting this PR, please make sure you have:
npm run testandnpm run lintand confirmed all checks still pass OR confirm CircleCI build passes