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

Docs: broken rerender on same name props' values #2460

Closed
gitname opened this issue Jan 24, 2018 · 10 comments
Closed

Docs: broken rerender on same name props' values #2460

gitname opened this issue Jan 24, 2018 · 10 comments

Comments

@gitname
Copy link
Contributor

gitname commented Jan 24, 2018

This comment was updated on January 30, 2018, to reflect the more general issue that included the behavior described in the original comment. You can see the original comment below the horizontal line.

Updated comment:

Here is a description of the more general issue:

In the docs, when I visit the "Icon" page and then visit the "Popup" page, I see the size options from the former appear on the latter. For example, the option "massive" appears on the "Popup" page, where it does not belong.

This behavior is demonstrated in the following screencast:

image


Original comment (created on January 24, 2018):

Steps

  1. Instantiate a Popup with a size of "massive"
    <Popup
      trigger={<Button icon='add' />}
      content='Add users to your feed'
      size='massive'
    />

Expected Result

The Chrome JavaScript console displays no warnings.

Actual Result

The Chrome JavaScript console displays the following warning:

image

Warning: Failed prop type: Invalid prop `size` of value `massive` supplied to `Popup`, expected one of ["mini","tiny","small","large","huge"].

Version

0.77.2

Testcase

Here's a demo of the issue:
https://codesandbox.io/s/101o69mrz7

@brianespinosa
Copy link
Member

@gitname the size of "massive" is not supported for the Popup component. The documentation does not show "massive" as a valid enumerated value for size on a popup: https://react.semantic-ui.com/modules/popup

image

This is because that in SUI, there are no styles or class for that value on a popup, which can be seen here: https://semantic-ui.com/modules/popup.html#size

@gitname
Copy link
Contributor Author

gitname commented Jan 24, 2018

Thanks for addressing this. You are correct: the Popup documentation does not show "massive" as a valid value for its size prop.

⚠️ EDIT: Sometimes it does! See next comment. ⚠️


When I created this issue, I was reading what I thought was the Popup docs, and saw "massive" listed as a valid value for the size prop. I even linked to the Popup docs in the demo I created. Now, I think I had been unknowingly reading the docs for a different component, which would have been my mistake, indeed.

Lessons learned:

  • Double-check the docs page I'm on
  • Include a screenshot in the Issue description

Sorry I did not do those things this time. I'll do them next time.

@gitname
Copy link
Contributor Author

gitname commented Jan 24, 2018

UPDATE:

Sometimes it does show it!

image

image

What in the world...?

😕


I'm out of time for now, but I'll come back to this later and try to come up with steps by which someone can reproduce this Docs behavior.

@brianespinosa
Copy link
Member

@gitname I will reopen this issue since it sounds like there may be an issue with the docs. Might be the way some data is getting reused in the views. Thanks for checking back to look at this and including screenshots.

@brianespinosa brianespinosa reopened this Jan 24, 2018
@gitname
Copy link
Contributor Author

gitname commented Jan 24, 2018

@brianespinosa I managed to reproduce this "inconsistent docs" behavior again just now by doing the following:

  1. Go to https://react.semantic-ui.com
  2. In the left menu, click on "Icon"
  3. Scroll down and see the presence of "massive" in the size list (as expected) ✅
  4. In the left menu, click on "Popup"
  5. Scroll down and see the presence of "massive" in the size list (not as expected) 🔥

On the other hand, when I did the above steps in the following order {1, 4, 5, 2, 3}, I saw "massive" missing from the "Icon" size list! It's as though the set of valid values from the first component I select, is used again by the second component I select.

I'll be out again for a few hours, and will check this Issue this when I return.

@gitname
Copy link
Contributor Author

gitname commented Jan 25, 2018

Here's a screencast GIF in which I demonstrate the issue:

2018-01-24_23-49-17

The screencast also contains a clue (I think) as to what's going on: I happened to select the size values on the "Icon" page. When I navigated to the "Popup" page, its size values were selected, too! It's as though the web browser considers them to be the same DOM elements.

@gitname
Copy link
Contributor Author

gitname commented Jan 25, 2018

Here's a screencast GIF that can serve as evidence that the web browser does consider them to be the same DOM elements.

2018-01-25_00-06-03

In this 40-second screencast, I modify element.style of the "small" <code> element on the "Icon" page, and it (surprisingly) affects the "small" <code> element on the "Popup" page in the same way!

@layershifter
Copy link
Member

I'm almost sure that problem is somewhere there, we use some HOCs that block rerenders, seems we have a problem there.

@layershifter layershifter changed the title Popup: docs list an incorrect size value Docs: broken rerender on same name props' values Jan 29, 2018
@gitname
Copy link
Contributor Author

gitname commented Feb 2, 2018

FYI: I still see the issue happening at https://react.semantic-ui.com/. That may be because, although you've fixed it in the code, the version of the code containing the fix hasn't been deployed to https://react.semantic-ui.com/ yet. I don't know whether that's the case.

@levithomason
Copy link
Member

Please submit a new issue with a complete report.

@Semantic-Org Semantic-Org locked as resolved and limited conversation to collaborators Feb 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants