Skip to content

#919, #881: Fixed 3rd party URLs-related issues #920

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

Merged
merged 2 commits into from
Apr 4, 2022
Merged

Conversation

kittaakos
Copy link
Contributor

@kittaakos kittaakos commented Mar 17, 2022

Motivation

This PR fixes various 3rd party URLs issues:

Change description

  • Single-sourced and fixed the way we parse a string to 3rd party URLs, added tests.
  • Adjusted the style of the dialog content display to fix the textarea height in the 3rd party URLs selector dialog.

Before:
Screen Shot 2022-03-17 at 14 18 00

After:
Screen Shot 2022-03-17 at 14 17 47

Other information

I am happy to split the PR into two, however, I thought it's easier to verify the 3rd party URLs changes in one shot. Let me know if a PR split is desired.

[ ] Docs have been added / updated (for bug fixes / features)

I do not know what this means. Thanks!

Reviewer checklist

@fstasi fstasi self-requested a review March 17, 2022 13:59
Copy link
Contributor

@fstasi fstasi left a comment

Choose a reason for hiding this comment

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

lgtm

@fstasi
Copy link
Contributor

fstasi commented Mar 17, 2022

Thanks @kittaakos for contributing, works like a charm to me.
@per1234 do you want to give this PR a try?

Copy link
Contributor

@per1234 per1234 left a comment

Choose a reason for hiding this comment

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

The File > Preferences > Additional Boards Manager URLs field no longer allows me to type a comma at the end of the line. This is problematic because it is a comma-separated list so a user who wants to add multiple URLs directly via the field will expect to be able to add a URL, then a comma, then the next URL.

  1. Select File > Preferences... from the Arduino IDE menus.
  2. Type a URL into the "Additional Boards Manager URLs" field.
  3. Type ,

🐛 The expected , character is not added. If you don't notice then the next URL will not be separated from the previous one.

It does allow commas to be added within existing text. It is only ignored when adding one at the end of the line.

@ubidefeo
Copy link

@kittaakos please address changes requested by @per1234 so I can approve this

@kittaakos
Copy link
Contributor Author

Thanks for the note. I did not forget about this. I will try to find some time to update the PR this week, @ubidefeo. The bug reported by Per is valid. I most likely need to change the <input> from a controlled React component to an uncontrolled one. To sum up, the fix is slightly bigger than I have expected.

Closes arduino#919.

Signed-off-by: Akos Kitta <kittaakos@gmail.com>
Closes arduino#881.

Signed-off-by: Akos Kitta <kittaakos@gmail.com>
@kittaakos
Copy link
Contributor Author

I have updated this PR. This time, I spent more time verifying my changes locally. Hopefully, I did not overlook anything and fixed the issue. Please review. Thank you!

Copy link
Contributor

@per1234 per1234 left a comment

Choose a reason for hiding this comment

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

Working perfectly for me now. Thanks Akos!

@kittaakos kittaakos changed the title #919, #881: Fixed 3rd part URLs-related issues #919, #881: Fixed 3rd party URLs-related issues Mar 28, 2022
@fstasi fstasi merged commit 0db119d into arduino:main Apr 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project
Projects
None yet
4 participants