jscheffl commented on PR #45270:
URL: https://github.com/apache/airflow/pull/45270#issuecomment-2567986116

   > Thanks @jscheffl for working on this. Had the first look. Looks great. The 
base is almost ready.
   > 
   > As mentioned in TODO, we will support the accordion and not flat.
   > 
   > Some points which i noticed, just writing it down incase we don't miss.
   > 
   >     1. There is no live changes to conf json onBlur
   > 
   >     2. Should have a validation for integer inputs.
   > 
   >     3. Dropdown not working for select values
   > 
   >     4. we should have a time picker.
   > 
   >     5. Length check not working
   >        ~6. JSON inputs should have a code view? (Just a opinion)~
   > 
   >     6. List should have multi lines
   > 
   > 
   > Overall this is going in right direction.
   > 
   > Should we have inbuilt scroll for the dynamic form? since the modal seems 
very long.
   > 
   > EDIT: Just saw a todo for point 6 (json code view) and most of the above 
issues as todo.
   
   (1) I recommend to make into a separate PR - as also discussed 1:1
   (2,3,5,6) fixed.
   (4) would need a component fix in react for Chrome... and a workaround for 
Firefox. Firfox time picking is "just broken" - would make this to a separate 
PR as well.
   
   So... for the Layout part I'd say... ready for review!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to