airbyte icon indicating copy to clipboard operation
airbyte copied to clipboard

Published 20 hours ago • verified publisher icon airbytehq

:window: :art: Connector Field UX Improvements

Open lmossman opened this issue 2 years ago • 7 comments

What

Relates to https://github.com/airbytehq/airbyte/issues/12204

This PR addresses the following parts of the above ticket:

  1. Moves field descriptions and examples into InfoTooltips (open to feedback on the tooltip/examples styling here!): Screen Shot 2022-09-01 at 6 49 01 PM
  2. Changes invalid input behavior so that instead of just highlighting the description, it adds the actual form error text below the input and uses the existing error prop logic on the inputs to make their border red: Screen Shot 2022-09-01 at 6 09 30 PM
  3. Removes the confusing * or lack-thereof after property labels which was meant to indicate optionality, and instead adds an Optional text to the right of the label for any optional fields.
    • Note 1: Some field names in the connector specs themselves contain * or (Optional) at the end - the connectors should be updated to remove these redundant suffixes now that we have logic in place to add the Optional text after the field. Should I programmatically try to filter these strings out of the field names in the meantime?
    • Note 2: I have explicitly set the optional flag on the PropertyLabel to false for the LabeledSwitch, ConditionSection, and ArraySection components, because it felt confusing as a user to have Optional on each of these when there is not really a way to not use them -- the switch is always on or off, condition always has some dropdown value selected, and even if the array is empty it is still submitted as a value. So to me, optional only really seemed to make sense on the basic text input components. Screen Shot 2022-09-01 at 6 11 14 PM
  4. Removes the logic to use the first field example as the placeholder text for its input, as suggested by Tim since he felt that it was confusing, making users think that they had to use the example value.

~TODO: modify the styling on Condition and Array section components to more closely match the styling in the original design, with the label being fully left-justified. This will allow us to hide the bounding line when there are no children in the condition group, since that case currently looks a bit strange now that the description has been moved into the InfoTooltip, e.g.: Currently just waiting on a final confirmation of the design from Nico before implementing this. This could also be done in a follow-up PR if the current state in this PR is acceptable.~ The styling of grouped fields has been updated to match the design provided by Nico: Screen Shot 2022-09-12 at 2 37 55 PM

NOTE: This PR also does not do any of the field grouping / optional fields hidden behind an expandable dropdown changes present in the design. Field grouping will require some more changes to connector specs, and hiding optional fields can be done in a follow-up PR.

How

There are several places that I have tried to refactor or simplify some components to reduce duplication and remove unneeded logic.

I also tried to make sure that the changes I've made here won't affect other views that are using the same components that I've touched, such as the connection Replication view. Though, the changes I've made here should hopefully make it easy to update those places to use the new styling as well (e.g. use the PropertyLabel class I have introduced to get the description-in-tooltip and Optional flag styling)

Recommended reading order

  1. PropertySection.tsx - changes to the base input properties
  2. PropertyLabel.tsx - replacement of the airbyte-webapp/src/views/Connector/ServiceForm/components/Property/Label.tsx component, since I was getting confused at the fact that it was named the same as the airbyte-webapp/src/components/Label/Label.tsx component which is also used further down the component tree.
  3. LabelInfo.tsx / LabelInfo.module.scss - replacement of the LabelMessage component, since this information is no longer being displayed in label "message" but instead in the InfoTooltip. Used by the PropertyLabel component to display an InfoTooltip with the field description and examples.
  4. ControlLabels.tsx - main change here is to add the InfoTooltip and the Optional pieces after the field label, which are both configured through new props so that this does not modify other views that use this component
  5. ConnectorNameControl.tsx - modified this to use the new PropertyLabel to get the same info tooltip styling as other inputs
  6. Control.tsx - just removed the example-placeholder logic, and added an error prop which is applied to the TextArea and Input subcomponents to utilize the existing logic they have to make input borders red when there is an input error
  7. The rest of the changed files are somewhat small and hopefully the diff is enough!

🚨 User Impact 🚨

Just visual changes to the connector page, but with all of the same functionality kept the same.

lmossman avatar Aug 30 '22 23:08 lmossman

@lmossman I updated Figma with a new component for the group of fields : https://www.figma.com/file/gMNRYVJav25pdsoxAyW7Qp/00_01_WEB-APP-LIBRARIES?node-id=1107%3A1880 - and here: https://www.figma.com/file/AxqFXuHo2BoTY0QY12hxm1/00_02_WEB-APP-COMPONENTS?node-id=446%3A8860 (you can change the width of this group of fields on Figma)

Upmitt avatar Sep 02 '22 09:09 Upmitt

@lmossman would it be possible to add storybooks for the areas impacted?

edmundito avatar Sep 02 '22 14:09 edmundito

Looks like there's a conflict!

krishnaglick avatar Sep 07 '22 15:09 krishnaglick

I've added storybooks for the main components that I touched (GroupControls and PropertyLabel). The positioning of the group controls border is slightly different in storybook than in the Airbyte UI for some reason - I will look at this with Tim tomorrow

lmossman avatar Sep 12 '22 23:09 lmossman

But I believe I have addressed all of the other outstanding things on this PR, so it should be ready for a final look @krishnaglick @edmundito @timroes @Upmitt

lmossman avatar Sep 12 '22 23:09 lmossman

Screen Shot 2022-09-13 at 9 47 23 AM The spacing seems off here. Also happens in the app.

krishnaglick avatar Sep 13 '22 13:09 krishnaglick

The spacing seems off here. Also happens in the app.

@krishnaglick yeah that is what I was seeing in storybook, as I called out here. But I wasn't seeing this spacing issue happen in the Airbyte UI -- it sounds like you were able to reproduce it there? If so, which connector did you use to reproduce it, and could you share a screenshot of that as well?

EDIT: I debugged this with Tim, it turned out that the storybook styling was being messed up due to <p> margins, which I have replaced with a div

lmossman avatar Sep 13 '22 16:09 lmossman

Screen Shot 2022-09-14 at 10 07 28 AM Could use just a little more spacing perhaps?

krishnaglick avatar Sep 14 '22 14:09 krishnaglick

Could use just a little more spacing perhaps?

@krishnaglick this PR is only focusing on the connector form, not the connection form. Did you maybe post the wrong screenshot?

lmossman avatar Sep 14 '22 17:09 lmossman

Could use just a little more spacing perhaps?

@krishnaglick this PR is only focusing on the connector form, not the connection form. Did you maybe post the wrong screenshot?

Ah! That makes a lot of sense. My bad! I'll retest the correct page.

krishnaglick avatar Sep 14 '22 17:09 krishnaglick

Some conflicts here, probably easy fixes from the React 18 upgrade prep PR.

krishnaglick avatar Sep 14 '22 18:09 krishnaglick

Some conflicts here, probably easy fixes from the React 18 upgrade prep PR.

Yep you were right - just resolved the conflicts and fixed the other places where I needed PropsWithChildren, so this should be good now

lmossman avatar Sep 14 '22 18:09 lmossman

Screen Shot 2022-09-15 at 9 43 45 AM Screen Shot 2022-09-15 at 9 45 27 AM

Source creation page. This is in Firefox. Looks cleaner on Chrome.

krishnaglick avatar Sep 15 '22 13:09 krishnaglick

Also this seems to have broken the the Authentication dropdown's styling. I'm happy to pair with you on this if you'd like!

krishnaglick avatar Sep 15 '22 13:09 krishnaglick

@krishnaglick I believe I've fixed all of the styling issues you pointed out above with these commits (tested on chrome, safari, and firefox -- all look the same now): https://github.com/airbytehq/airbyte/pull/16152/files/e63e414b3fe45ebf0540d06df7c3c3dbed75d5ff..d39a2447e1b5662984a50a937ff1de615cf59eef

I still scheduled some time with you tomorrow to go over some of those changes over voice, to make sure I'm not doing anything weird. But if those changes look good to you then this may be ready to go

lmossman avatar Sep 15 '22 23:09 lmossman