Skip to content

Amend the manual network configuration "subnet" field label to clarify it's the network address that's requested#2259

Open
dwbfox wants to merge 1 commit intocanonical:mainfrom
dwbfox:dag-ipfield-subnetmask-fix
Open

Amend the manual network configuration "subnet" field label to clarify it's the network address that's requested#2259
dwbfox wants to merge 1 commit intocanonical:mainfrom
dwbfox:dag-ipfield-subnetmask-fix

Conversation

@dwbfox
Copy link

@dwbfox dwbfox commented Oct 8, 2025

The "Subnet" prompt displayed in the manual/static network configuration screen has a prompt to provide the network address + cidr (i.e. 192.168.0.0/24) in a specific notation.

However, the field label just asks for Subnet:, which is misleading as 255.255.255.0 is a valid subnet but gets rejected.

Moreover, the error message that is displayed when a user enters an invalid entry, should be in CIDR form (xx.xx.xx.xx/yy), is equally vague as it doesn't indicate the requested subnet is actually the first IP of the subnet of the desired IP address of the machine (i.e. the network address).

@dbungert
Copy link
Member

dbungert commented Oct 9, 2025

Hi - thanks for the PR.

A few things:

  • Please push your changes from your @canonical.com email, so that cla-check is happy
  • It's better to modify the underlying string than the en_US string directly, so that we can convey the intention to all translators. They will be unlikely to reference the en_US translation specifically on this. Fortunately the .po file lists the underlying location of the string - subiquitycore/ui/views/network_configure_manual_interface.py:76

Now the content itself - we have a few fields today

  • Subnet
  • Address
  • Gateway

for a 192.168.100.20 address on the 192.168.100.0/24 subnet with gateway 192.168.100.1, we'd expect these inputs

  • 192.168.100.0/24
  • 192.168.100.20
  • 192.168.100.1

Calling the first field "Network address CIDR" is an improvement in the sense that we list the CIDR part. But when I read "Network address CIDR" I think I'm being asked for 192.168.100.20/24, which is wrong for at least 2 reasons.

What do we think about the label "CIDR Block" ? cc @ogayot @Chris-Peterson444

@ogayot
Copy link
Member

ogayot commented Oct 9, 2025

What do we think about the label "CIDR Block" ? cc @ogayot @Chris-Peterson444

I agree that the current implementation is misleading. I think my preference would be to drop the subnet field entirely and replace it with a subnet mask noted as /xx:

Something like:

Before After
Screenshot from 2025-10-09 15-23-23 Screenshot from 2025-10-09 15-23-23-edited

I can't think of a good reason for requiring the network address to be specified by the user. We can calculate it if needed (and we already do - sort of - since we're validating that the local address belongs to the network.

But this might be too involved for the scope of the original PR.

@dwbfox
Copy link
Author

dwbfox commented Oct 9, 2025

What do we think about the label "CIDR Block" ? cc @ogayot @Chris-Peterson444

I agree that the current implementation is misleading. I think my preference would be to drop the subnet field entirely and replace it with a subnet mask noted as /xx:

Something like:
Before After
Screenshot from 2025-10-09 15-23-23 Screenshot from 2025-10-09 15-23-23-edited

I can't think of a good reason for requiring the network address to be specified by the user. We can calculate it if needed (and we already do - sort of - since we're validating that the local address belongs to the network.

But this might be too involved for the scope of the original PR.

I really like this design, much clearer

@dbungert
Copy link
Member

dbungert commented Oct 9, 2025

What do we think about the label "CIDR Block" ? cc @ogayot @Chris-Peterson444

I agree that the current implementation is misleading. I think my preference would be to drop the subnet field entirely and replace it with a subnet mask noted as /xx:

An interesting suggestion! I like this for people who know was is asked for with the CIDR notation, but if you don't know what's expected after the slash it will be complicated.

But this might be too involved for the scope of the original PR.

Yes, a minor UX redesign like this is well out of scope for fixing a label.

@Chris-Peterson444
Copy link
Contributor

What do we think about the label "CIDR Block" ? cc @ ogayot @ Chris-Peterson444

Yeah this is an improvement to the existing field I think. My default interpretation is to pass a subnet mask instead of the CIDR format at first glance.

I agree that the current implementation is misleading. I think my preference would be to drop the subnet field entirely and replace it with a subnet mask noted as /xx:

Yeah this an interesting approach! I think we'd have to add some text to be explicit about what we want here, but it feels like a good goal. I agree it's out of scope for the current PR but we should document this for future improvements.

I can't think of a good reason for requiring the network address to be specified by the user.

For manual/non-dhcp setup, wouldn't we want to allow the user to specify a static IP?

@ogayot
Copy link
Member

ogayot commented Oct 10, 2025

but if you don't know what's expected after the slash it will be complicated.

Right, but this is more or less the same situation today isn't it. If you're not familiar with the /xx notation, you can't properly set the "Subnet" field.

For manual/non-dhcp setup, wouldn't we want to allow the user to specify a static IP?

The static host IP, yes. But if you have a static host IP and a subnet length, the network address is redundant.

import ipaddress
ipaddress.IPv4Interface("10.0.2.15/24").network
> IPv4Network('10.0.2.0/24')

AFAICT, it only makes sense to specify the network address if we don't want a host address. But this isn't supported.

@Chris-Peterson444
Copy link
Contributor

The static host IP, yes. But if you have a static host IP and a subnet length, the network address is redundant.

Ahhh right this makes sense. I was getting this backwards originally, thanks for the explanation! I can see how the single address field + subnet bit length field would be pretty clean now.

@dwbfox
Copy link
Author

dwbfox commented Oct 14, 2025

Should I abandon this PR, as it sounds like this may be fixed with a more comprehensive change to the UX in this screen.

@ogayot
Copy link
Member

ogayot commented Oct 15, 2025

Should I abandon this PR, as it sounds like this may be fixed with a more comprehensive change to the UX in this screen.

I'm happy to see the label changed in the meantime. I think "CIDR block", as Dan suggested, might be a better alternative than "Network Address CIDR" though.

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants