-
-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
fixes issue #7366: Adjust Option Orientation in CreateRadio() reference. #7367
Conversation
🎉 Thanks for opening this pull request! Please check out our contributing guidelines if you haven't already. And be sure to add yourself to the list of contributors on the readme page! |
is this change appropriate? |
Thanks @Prajyot05 for your work on this. @devesh-2002, Do you want to review this code, since you were interested in solving this issue and it was assigned to you previously. |
@perminder-17 are any changes required or should I raise a PR? |
@perminder-17 any updates? |
I'll review it today. Thanks for your work and your patience. Been really busy this week. So sorry for the delay. |
@perminder-17 No problem sir, I hope I can help solve the issue |
src/dom/dom.js
Outdated
@@ -1535,9 +1548,10 @@ p5.prototype.createSelect = function(...args) { | |||
* createCanvas(100, 100); | |||
* | |||
* // Create a radio button element and place it | |||
* // in the top-left corner. | |||
* // in the top-right corner. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here as well, it should be top-left corner.
oh ok, I thought that was the position of the radio button. I'll fix that, are the rest of the changes fine? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your patience and your work. Looks good to me. Could you please make this PR ready for the review by removing it from "draft".
@perminder-17 thanks, sure I'll make it PR ready. |
@davepagurek are there any changes I need to make? |
@perminder-17 any updates on this? |
I think this is good for now! Ideally there would be a way to do this without having to create a custom style tag like this, but it does seem like maybe the best option currently. Maybe worth making a new issue to make the default behaviour nicer? |
Resolves #7366
Changes:
Added styling to the radio, hence aligning them one below the other.
Screenshots of the change:
PR Checklist
npm run lint
passes