Skip to content
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

Introduce DapperScrollbars #12961

Merged
merged 13 commits into from
Mar 28, 2019
Merged

Introduce DapperScrollbars #12961

merged 13 commits into from
Mar 28, 2019

Conversation

alexpaxton
Copy link
Contributor

@alexpaxton alexpaxton commented Mar 27, 2019

Closes #12903
Closes #12630
Closes #12629
Closes #12989

Changes

Introduce DapperScrollbars wrapper for react-scrollbars-custom package as a replacement for FancyScrollbars & react-custom-scrollbars

Implement DapperScrollbars in higher level components as well as areas that were problematic with FancyScrollbars

Style DapperScrollbars to mostly match the appearance of FancyScrollbars except for that the scrollbar track is slightly visible in DapperScrollbars (useful for discerning which scroll component is being used at a glance).

Preview

dapper-scrollbars
Screen Shot 2019-03-27 at 4 35 14 PM

Gotchas

  • Partially because the underlying library works a little differently, dropdowns with a menu that is wider than the button must now be explicitly passed a width rather than it being automatically determined. After reviewing the UI this only is needed in a few places.

Tech Debt:

  • Replace FancyScrollbars with DapperScrollbars #12960 (Replace FancyScrollbars with DapperScrollbars so we're not supporting both)
  • Sometimes the scrollbar flickers before disappearing on dropdown menus that don't have enough contents to scroll
  • All the styles to customize the appearance of DapperScrollbars are relying on !important to override some of the inline styles injected by the library
    • The library has an option to not inject any inline styles, which allows for more customization, but it doesn't work as expected

Checklist:

  • CHANGELOG.md updated with a link to the PR (not the Issue)
  • Rebased/mergeable
  • Tests pass
  • http/swagger.yml updated (if modified Go structs or API)
  • Sign CLA (if not already signed)

type Props = PassedProps & DefaultProps

@ErrorHandling
class DapperScrollbars extends Component<Props> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be PureComponent?

Copy link
Contributor

@AlirieGray AlirieGray left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

@OfTheDelmer
Copy link
Contributor

Would this help with any of these: #12989 #12987 #12986

@alexpaxton
Copy link
Contributor Author

@OfTheDelmer it would close #12989

@alexpaxton alexpaxton merged commit 133b131 into master Mar 28, 2019
@alexpaxton alexpaxton deleted the feat/dapper-scrollbars branch March 29, 2019 00:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants