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

Pagination: Received NaN for the 'value' attribute #4035

Closed
gcalica opened this issue Aug 15, 2020 · 4 comments · Fixed by #4037
Closed

Pagination: Received NaN for the 'value' attribute #4035

gcalica opened this issue Aug 15, 2020 · 4 comments · Fixed by #4037

Comments

@gcalica
Copy link
Contributor

gcalica commented Aug 15, 2020

Bug Report

When you you don't specify an activePage or a defaultActivePage; then on the first render of the Pagination, two of the Pagination Items (Previous Item and Next Item) have "NaN" as their values

Steps

A clear and concise description of steps to reproduce the problem.

const App = () => <Pagination totalPages={5} />;

Expected Result

The result that you expected.

For the Pagination to not throw a warning in the console.

Actual Result

The actual result that happened 💣

Throws this warning in the console

Warning: Received NaN for the `value` attribute. If this is expected, cast the value to a string.
    in a (created by MenuItem)
    in MenuItem (created by PaginationItem)
    in PaginationItem (created by Pagination)
    in div (created by Menu)
    in Menu (created by Pagination)
    in Pagination (created by App)
    in App

Version

"react": "^16.13.1",
"semantic-ui-react": "^1.2.0"

Testcase

[Fork, update, and replace this pen to show the bug]:

https://codesandbox.io/s/semantic-ui-react-forked-76upj?file=/index.js

@welcome
Copy link

welcome bot commented Aug 15, 2020

👋 Thanks for opening your first issue here! If you're reporting a 🐞 bug, please make sure you've completed all the fields in the issue template so we can best help.

We get a lot of issues on this repo, so please be patient and we will get back to you as soon as we can.

@layershifter
Copy link
Member

layershifter commented Aug 15, 2020

@gcalica good catch, thanks for reporting 👍 It happens because activePage in Pagination's state defaults to undefined if there is no activePage/defaultActivePage. I suggest to set a default value in a component itself:

src/addons/Pagination/Pagination.js

export default class Pagination extends Component {
+  getInitialAutoControlledState() {
+    return { activePage: 1 }
+  }

test/specs/addons/Pagination/Pagination-test.js

  common.isConformant(Pagination, {
    requiredProps: {
      totalPages: 0,
    },
  })
  common.hasSubcomponents(Pagination, [PaginationItem])

+  describe('activePage', () => {
+    it('defaults to "1"', () => {
+      const wrapper = mount(<Pagination totalPages={3} />)
+
+      wrapper.find('PaginationItem').at(0).should.have.prop('active')
+    })
+
+    it('can be set via "defaultActivePage"', () => {
+      const wrapper = mount(<Pagination defaultActivePage={2} totalPages={3} />)
+
+      wrapper.find('PaginationItem').at(3).should.have.prop('active')
+    })
+
+    it('can be set via "activePage"', () => {
+      const wrapper = mount(<Pagination activePage={2} totalPages={3} />)
+
+      wrapper.find('PaginationItem').at(3).should.have.prop('active')
+    })
+  })
+

Would you like to a submit a PR? 😸

@gcalica
Copy link
Contributor Author

gcalica commented Aug 15, 2020

@layershifter Submitted a PR.

@layershifter
Copy link
Member

A fix was released in [email protected] 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants