DEV Community

Leyang Yu
Leyang Yu

Posted on

Hacktoberfest Week 3

Intro

For the third week of Hacktoberfest, I worked on the Fluent UI repository. As a primarily front-end developer, React UI component libraries are something that have made my life so much easier so I was really interested in working on and contributing to this repository.

The issue
The pull request

The Issue

The issue at hand was that for the Fluent UI react-northstar library, there was a small UI bug for dropdowns.

Search Multiple

As you can see in the image above, when the input spans multiple lines, a scrollbar will appear and overlap with the dropdown indicator.

My Approach

In terms of getting started, it was quite straightforward and did not require too much setup. All I had to do was run yarn and yarn start, and choose @fluentui/docs from the command line to run the demo site locally:

@fluentui/docs

Next, I began to think of how to approach this problem. First, I looked at other React component libraries, such as reactstrap and React-Select to get some ideas for how to approach this issue.

reactstrap
reactstrap

React-Select
React-Select

Personally, I really like the way that the React-Select multiple select dropdown is styled. There is no scrollbar so this eliminates the issue of having a scrollbar overlap with the indicator.

Next, I looked at how the dropdown was currently styled. This is what I found:

selectedItems: ({ props: p, variables: v }): ICSSInJSStyle => ({
    ...
    overflowY: 'auto',
    overflowX: 'hidden',
    maxHeight: v.selectedItemsMaxHeight,
    ...
  }),
Enter fullscreen mode Exit fullscreen mode

In my pull request, the change I made was to remove the scrollbar by setting overflow to be hidden and removing the max height of the dropdown container. This follows the same style as the React-Select dropdown and you can see the code and end results below:

selectedItems: ({ props: p, variables: v }): ICSSInJSStyle => ({
    ...
    overflow: 'hidden',
    //maxHeight: v.selectedItemsMaxHeight, // removed this line
    ...
  }),
Enter fullscreen mode Exit fullscreen mode

And the result of these changes:
Search Multiple After

To be honest, this fix was much simpler than I had expected. However, I wasn't sure if the code maintainers also liked this style so in my pull request, I suggested some other alternatives to the approach I took.

1st Alternative: Another idea would be to keep the scrollbar and move the indicator left, to prevent them from overlapping. However, this would also affect the positioning of the indicator even for dropdowns with no scrollbar, which does not seem like an ideal solution.

2nd Alternative: The last idea I had would be to still keep the scrolling functionality but to just hide the scrollbar (set the scrollbar width to 0). However, I was worried about whether this idea would be user-friendly.

So far, I have not yet received any reviews on my pull request, but nonetheless I enjoyed working on this issue and look forward to the last week of Hacktoberfest!

Oldest comments (0)