Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clickable labels for checkboxes #741

Merged
merged 5 commits into from
Oct 17, 2021
Merged

Clickable labels for checkboxes #741

merged 5 commits into from
Oct 17, 2021

Conversation

AC4BB21B
Copy link
Contributor

A lot of checkboxes had unclickable labels, i.e. to toggle them you had to click on the box (clicking on the legend would achieve nothing).

In this PR, I added labels for checkboxes and radio buttons in:

  • upload
  • bulk download
  • security settings
  • add friends

On a side note, I tried to conform to each file's indentation (some have 4 spaces, some 2, and mostly some appear mixed). Sadly, Visual Studio Code gets a bit rebellious with mixed-indented files, and my commits aren't as perfectly aligned as they looked in VSCode :/

@ArneBab
Copy link
Contributor

ArneBab commented Sep 29, 2021

Aside from the one comment, this looks good to me — and is a great usability improvement. Thank you!

@AC4BB21B
Copy link
Contributor Author

AC4BB21B commented Sep 29, 2021

Actually, I spotted some other forms that I missed in the wizard, mind if I label those as well in this PR?

@ArneBab
Copy link
Contributor

ArneBab commented Sep 30, 2021

Feel free to do that — thank you!

@AC4BB21B
Copy link
Contributor Author

This time I think that's it. The unlabelled checkboxes that I still saw are from plugins

new String[] { "radio", "plugin-name", pluginDescription.name, "radioPlugin" + pluginDescription.name });
option.addChild("label",
new String[] { "for" },
new String[] { "radioPlugin" + pluginDescription.name }).addChild("i", pluginDescription.getLocalisedPluginName());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you add a newline before .addChild("i")?

Copy link
Contributor Author

@AC4BB21B AC4BB21B Oct 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better like

option.addChild("label",
  new String[] { "for" },
  new String[] { "radioPlugin" + pluginDescription.name })
  .addChild("i", pluginDescription.getLocalisedPluginName());

or rather like

option.addChild("label",
  new String[] { "for" },
  new String[] { "radioPlugin" + pluginDescription.name }
).addChild("i", pluginDescription.getLocalisedPluginName());

or even

option.addChild("label",
  new String[] { "for" },
  new String[] { "radioPlugin" + pluginDescription.name }
).addChild("i",
  pluginDescription.getLocalisedPluginName()
);

I saw a bit of everything around, so I had a hard time picking the favored choice, I rather tried to be more or less the same as what was nearby - unless it was clearly too unreadable, like some one-liners

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’d say the second: that is what the code around it does.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in f498203

@ArneBab
Copy link
Contributor

ArneBab commented Oct 2, 2021

Looks good to me — thank you very much!

@ArneBab ArneBab merged commit 48b799f into hyphanet:next Oct 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants