-
Notifications
You must be signed in to change notification settings - Fork 63
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
feat: enabled manual height adjustment for bounty description box #1299
feat: enabled manual height adjustment for bounty description box #1299
Conversation
Great work. Screenshot looks good. All checks passed. Let's do a code review. |
@@ -444,8 +444,9 @@ function Form(props: FormProps) { | |||
.map((item: FormField) => ( | |||
<Input | |||
{...item} | |||
type={item.name === 'description' ? 'textarea' : item.type} |
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.
instead of cheking item.name === 'description'
twice lets define it as a boolean above and use it in the 3 places
@@ -97,7 +97,7 @@ export default function TextAreaInput({ | |||
}} | |||
className={active ? 'euiFormRow_active' : (value ?? '') === '' ? '' : 'euiFormRow_filed'} | |||
border={borderType} | |||
label={labeltext} | |||
label={labeltext === 'Description' ? `${labeltext} *` : labeltext} |
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.
we should try to keep logic that is not related to the common component out of here
@kevkevinpal alright! |
All checks pass. Great job. @elraphty let's get a code review. |
@okhot I can't seem to increase the description height on my chrome browser on Mac Loom Video |
@elraphty Thanks for the Mac testing. Could you please double-check if you are testing the correct branch? Unfortunately, I don't have access to a Mac for detailed investigation. Any specific logs you provide will be helpful. Thanks |
@okhot it works on firefox, but doesn't work on chrome Loom Video, make it work on chrome. |
It doesn't work on Mac's chrome, I think you have to make some research regarding this |
Nice Job @okhot, I tested it on my Chrome Mac and it works now. I'm merging right away. |
tested and it works. GJ! Paid! |
…or-bounty-description-box feat: enabled manual height adjustment for bounty description box
…or-bounty-description-box feat: enabled manual height adjustment for bounty description box
Describe your changes
enabled manual height adjustment for bounty description box
Issue ticket number and link: #1148
Preview
https://www.loom.com/share/9a26a03ecaf94ad0bdc2d1d222933f4f?sid=803ca758-716e-4ca4-a4e4-fc498fae871d
Type of change
Feature
Please delete options that are not relevant.
Checklist before requesting a review