Make SideModal submit button copy consistent

[charliepark]

Fixes #984

We had a few different strings we were using for the various edit forms. This PR aligns them, by adding a new formType ('create' | 'edit') prop. For edit forms, we have a default button label of "Save changes" (which we were already using in a few places).

This PR also removes an id prop, which we can generate dynamically, allowing us to clean the props up a little.

I'm ambivalent on whether formType should be a required prop, as we're only using it to set the default label on edit forms. We could make it optional and clean up the various create form callsites.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
console	✅ Ready (Inspect)	Visit Preview	Mar 11, 2024 6:40pm

[david-crespo]

Ha, @benjaminleonard was way ahead of us on the form type concept:

Just a thought – in order to keep this consistent going forward, is it possible to pass a pane type (e.g. Create / Edit / Delete) to the form, and have it by default set the text. And we could use the same patterns for title + submit text too?

[david-crespo]

I'm on the fence too, but I think formType should be required so you have to make a decision rather than, for example, accidentally leaving it off on an edit form and getting the create behavior by default.

[david-crespo]

I like the change — it fixes the inconsistency and reduces choices we have to make at dev time.

It's a small thing, but since we're giving all the edit forms the same button label, I want to make sure we considered all possibilities. The plausible options I can think of are:

Label	Comment
Save changes	Fine, though something about "changes" bothers me. "Save" makes me think of saving progress for later rather than submitting my update permanently
Save	Kinda short, plus same problem as "save" above
Update	A little better than "save", I think, but similar
Update [resource]	I might like this the best, but it's also the most annoying. We could do it by making the edit form take a resource name prop and have it plug it into both the title with Edit ${resource} and the button label with `Update ${resource}

[charliepark]

I'm on board with the Edit/Update ${resource} option. Adding the resource prop won't be too painful, and I think it'll give the best effect. Will pick that up in the morning.

[david-crespo]

I’d add a doc comment here explaining that you only need this when you want to override the default create or edit title. Same with submitLabel I think.

[david-crespo]

“Change user role” is a little more descriptive? I could go either way. If it shows up for editing a group role too it’s misleading.

[charliepark]

Yeah, I agree that Change is probably more accurate than Edit, as they aren't editing the role, per se. Change permissions?

[charliepark]

On the actions modal we say Change role, so I think that makes sense. Should we determine whether it's a user or group and pass in the appropriate term? Change user role / Change group role

[david-crespo]

Permissions isn’t a term in the API, so I prefer to avoid it. Really what they’re doing in this case is updating the policy on the resource, and the UI is sort of deliberately obscuring that (uncharacteristically) because it is so much more natural to think about operating on the user.

[charliepark]

Hold the phone. There was a TODO in the project-access and silo-access files noting that we should // TODO: show user name in header or SOMEWHERE … this seems like as good a time as any to update that in the modal's title.

[david-crespo]

Yea, it’s a bit weird but it’s still an improvement. See what it looks like with a 63 character name.

[charliepark]

Though it worked if the name was hypenated, it overran the boundary without hyphens …

… so I added word-break: break-all to the class for that header:

[charliepark]

Though …

I have an idea I'll try here.

[david-crespo]

what about break-words instead of break-all?

[charliepark]

I tried that, plus wrapping a container in display: inline-block and individual words in both break-words and break-all, but neither worked as desired 100% of the time. I think users will run into more situations with something like Name ofPerson or hyphenated-group-name as opposed to unhyphenatedLongGroupName, so break-words is probably the best solution for now.