Add workerPool (née workerType) creation interface (non-working)

[owlishDeveloper]

Bugzilla Bug: 1554798

workerType is now workerPool in worker manager universe. I'd like to rename All The Things in a separate PR on a separate branch tho, so this PR has the old terminology <-- changed my mind on this to make the work more modular. This PR contains all the new terminology. The rest comes in a separate PR

[helfi92]

This is looking really good! I added some requested changes. Happy to discuss if you disagree with anything.

[helfi92]

Looking pretty solid. Added some final requested changes :)

[helfi92]

Instead of indenting the provider name and provider type

we can put everything in one line:

<Grid container>
  <Grid item xs={12} sm={6}>
    <ListItem>
      <TextField
        fullWidth
        label="Name"
        value={workerPool.providerId}
        name="providerId"
        onChange={this.handleInputChange}
      />
    </ListItem>
  </Grid>
  <Grid item xs={12} sm={6}>
    <ListItem>
      <TextField
        className={classes.dropdown}
        id="select-provider-type"
        select
        label="Type"
        helperText="Which service do you want to run your tasks in?"
        value={workerPool.providerType}
        name="providerType"
        onChange={this.handleInputChange}>
        {Object.keys(providers).map(p => (
          <MenuItem key={p} value={p}>
            {p}
          </MenuItem>
        ))}
      </TextField>
    </ListItem>
  </Grid>
</Grid>

Other edits in the code block:

• I removed margin="normal" from both text fields in order to bring them closer to the list subheader "Provider".'
• s/Type:/Type/
• Added fullWidth to the first TextField

[owlishDeveloper]

we can put everything in one line

how is this better than indenting? If one uses trackpad or a mouse, indented fields require less movement as they are grouped more closely together. And I just like the indentation better :)

[djmitche]

It might not matter: the user doesn't get to specify the provider type here, just the provider ID. There's no such thing as "provider name".

[helfi92]

I don't think it is necessary to use indentation when there is a clear subheader that does the job of separation. Also, I think Indented text fields don't sit well on mobile.

[owlishDeveloper]

the provider type here, just the provider ID. There's no such thing as "provider name".

We don't have to leak the names of DB fields into the UI, and the reason I didn't change it - in my mind ID is usually a numeric or a UUID-type of string. My intention was to make it clearer that user can enter an understandable thing in there, like a name.

That said, if you think "provider ID" is better than "provider name", I can make that change.

[owlishDeveloper]

Sorry, updated version: (and thank you for uncovering the padding issue on mobile, @helfi92 !)

[helfi92]

Looks a lot better than before. I still think the indentation is more suitable for things like tree views, radio buttons, etc. I think Indentation is not necessary for inputs when there is a subheader above it. If we are trying to add more emphasis to it, then we can try adding more vertical spacing on top of list subheaders. Let me know if that does the job.

[owlishDeveloper]

If we are trying to add more emphasis to it

Yes, that's exactly what I am trying to do. Subheaders don't work for me because they're small font (even smaller than those label/placeholder hybrids), and the whole thing just looks like a uniform wall of text to me.

I'll try experimenting with vertical spacing

[owlishDeveloper]

This spacing works for me:

[owlishDeveloper]

(it's in the last commit I pushed)

[helfi92]

I like the new design a lot! I think we should also incorporate the spacing technique in our other forms. I've filed #835. Just some minor nits then we should be ready to merge.

[helfi92]

🎉 🎊 Added some comments but feel free to ignore. Great work.

[helfi92]

That's probably a bit too much padding. I think equal padding works but I'll let you decide on that.

[owlishDeveloper]

To my eye, when it's equal, the lower one seems smaller (it's actually a thing about human vision - if you noticed the shape of capital B in many fonts, for example; the upper part is often smaller to make the letter look balanced) - I planned to commit 70 and 90 pixels yesterday, but for some reason the previous iteration ended up in the commit

[helfi92]

I can't edit this field via the UI. workerPoolId probably is not listed in the state.

[owlishDeveloper]

it will be! The fields will be corrected when I start working on the actual functionality of the form