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

[Project] Update core components storybook doc #253

Merged
merged 69 commits into from
Feb 17, 2020

Conversation

chenesan
Copy link
Contributor

Purpose

Merge new storybook docs(#246 #247 #248 #249 #251 #252) into develop branch.
Also fix anchored popover position in #249 .

Asana: https://app.asana.com/0/347798529122928/1156749765218241

I will open this PR once the above PRs are into the project branch.

Changes

  • Revise packages/storybook/examples/core with new storybook addon-docs
  • Fix anchored popover top position when document is scrollable

Risk

None.

TODOs

  • Describe what should be done outside of this PR
  • Maybe in other PRs or some manual actions.

There are 2 questions in previous modal stories:

1. All the modals is default to open(some are not closable), which will block the docs page.

2. we cannot see the implementation in "Show code" section. You can see <ClosableExample> but can't know how it work inside through storybook, unless reading the source code.

This commit try to solve them by:

1. Use `<ClosableModal>` (Rewrite as functional component) as basic usage.
2. In each example, we just use `<ClosableModal>`, add comment to tell user read the closable modal example code.

I know the result is not satisfying -- It's still hard to read the code at a glance. But at least it solve question (1) and also help question (2) a little.

code section.
Anchored popover is a <Popover> wrapped with two HOCs:
`closable()(anchored()(Popover)`

Before this commit it will form following DOM structure:

```html
<body>
  <div class="gyp-base-layer">
    <div class="gyp-closable>
      <div class="gyp-popover gyp-popover--top">
    </div>
  </div>
</body>
```

`gyp-popover` is absolutely positioned, with `top` value which
is relative to its document top left corner. `gyp-closable` is
`position: fixed` with `width: 100vw` and `height: 100vh`.

It works when document is too short to scroll, such as previous
storybook doc canvas. But when the element triggering popover is
over the screen(i.e. we have to scroll to click it), the position
of anchored popover will be wrong because the `position: absoulte`
of `gyp-popover` is relative to `gyp-closable`, which is
`position: fixed`, so the `top` value is wrong.

The position of `gyp-popover` should always be relative to the
document. So in this fix we change the `closable` HOC code to change
DOM structure as:

```html
<body>
  <div class="gyp-base-layer">
    <div class="gyp-closable" />
    <div class="gyp-popover" />
  </div>
</body>
```

Let the `gyp-popover` be relative to `gyp-base-layer`, which is absolute
to `document.body`.

Note that this bug won't happen on Cloud2 since on Cloud2 the
`document.body` height is always `100vh`, which is not scrollable.
Revamp core components storybook doc(pt. 1)
Revamp core components storybook doc(pt. 2)
@chenesan chenesan self-assigned this Feb 10, 2020
@chenesan chenesan changed the title [Project]Update core components storybook doc [Project] Update core components storybook doc Feb 10, 2020
@chenesan chenesan marked this pull request as ready for review February 14, 2020 07:20
Copy link
Contributor

@zhusee2 zhusee2 left a comment

Choose a reason for hiding this comment

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

go go 📚

Copy link
Contributor

@benny0642 benny0642 left a comment

Choose a reason for hiding this comment

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

Go 🚀

@chenesan chenesan merged commit c464ab0 into develop Feb 17, 2020
@chenesan chenesan deleted the project/update-core-components-storybook-doc branch February 17, 2020 10:24
@kyoyadmoon kyoyadmoon mentioned this pull request Feb 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants