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.

Sign up for GitHub

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

Jump to bottom

Add previous / next iteration buttons #418

Merged
merged 1 commit into from Jan 13, 2021
Merged

Add previous / next iteration buttons #418

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 19 additions & 0 deletions app/javascript/components/mentoring/discussion/IterationsList.tsx
View file
Open in desktop
Expand Up @@ -11,6 +11,8 @@ export const IterationsList = ({
current: Iteration
onClick: (iteration: Iteration) => void
}): JSX.Element => {
const currentIndex = iterations.indexOf(current)

return (
<nav className="iterations">
{iterations.map((iteration) => (
Expand All @@ -21,6 +23,23 @@ export const IterationsList = ({
selected={current === iteration}
/>
))}

<button
type="button"
aria-label="Go to previous iteration"
onClick={() => onClick(iterations[currentIndex - 1])}
disabled={iterations[0] === current}
>
Previous
</button>
<button
type="button"
aria-label="Go to next iteration"
onClick={() => onClick(iterations[currentIndex + 1])}
disabled={iterations[iterations.length - 1] === current}
>
Next
</button>
Comment on lines +27 to +42
Copy link
Member

Choose a reason for hiding this comment

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

This is fine but this is technically pagination and thus it could be implemented as:

<a
  href={urlThatGoesThere}
  onClick={handlerThatPreventsDefaultAndGoesThere}
  rel="next"

That way, users can mid or right click and store the url to the iteration.

Also, you probably want to focus the new/linked iteration after "opening" it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@SleeplessByte I'd like to do this as well. What's a good approach to update the URL link once we click on the button?

I'm merging this PR in for now, and I'll open a separate PR to do that.

</nav>
)
}
93 changes: 93 additions & 0 deletions test/javascript/components/mentoring/Discussion.test.js
View file
Open in desktop
Expand Up @@ -254,3 +254,96 @@ test('switches tabs', async () => {
screen.queryByRole('tabpanel', { name: 'Discussion' })
).not.toBeInTheDocument()
})

test('go to previous iteration', async () => {
const links = {
scratchpad: 'https://exercism.test/scratchpad',
exercise: 'https://exercism.test/exercise',
posts: 'https://exercism.test/posts',
}
const track = {
title: 'Ruby',
}
const exercise = {
title: 'Bob',
}
const student = {
avatarUrl: 'https://exercism.test/avatar',
}
const iterations = [
{
idx: 1,
links: {
files: 'https://exercism.test/iterations/1/files',
},
},
{
idx: 2,
links: {
files: 'https://exercism.test/iterations/2/files',
},
},
]

render(
<Discussion
exercise={exercise}
links={links}
track={track}
student={student}
iterations={iterations}
discussionId={1}
/>
)
userEvent.click(
screen.getByRole('button', { name: 'Go to previous iteration' })
)

expect(await screen.findByText('Iteration 1')).toBeInTheDocument()
})

test('go to next iteration', async () => {
const links = {
scratchpad: 'https://exercism.test/scratchpad',
exercise: 'https://exercism.test/exercise',
posts: 'https://exercism.test/posts',
}
const track = {
title: 'Ruby',
}
const exercise = {
title: 'Bob',
}
const student = {
avatarUrl: 'https://exercism.test/avatar',
}
const iterations = [
{
idx: 1,
links: {
files: 'https://exercism.test/iterations/1/files',
},
},
{
idx: 2,
links: {
files: 'https://exercism.test/iterations/2/files',
},
},
]

render(
<Discussion
exercise={exercise}
links={links}
track={track}
student={student}
iterations={iterations}
discussionId={1}
/>
)
userEvent.click(screen.getByRole('button', { name: 'Go to iteration 1' }))
userEvent.click(screen.getByRole('button', { name: 'Go to next iteration' }))

expect(await screen.findByText('Iteration 2')).toBeInTheDocument()
})
26 changes: 26 additions & 0 deletions test/javascript/components/mentoring/discussion/IterationsList.test.js
View file
Open in desktop
@@ -0,0 +1,26 @@
import React from 'react'
import { render, screen } from '@testing-library/react'
import '@testing-library/jest-dom/extend-expect'
import { IterationsList } from '../../../../../app/javascript/components/mentoring/discussion/IterationsList'

test('next iteration button is disabled when on last iteration', async () => {
const current = { idx: 2 }
const iterations = [{ idx: 1 }, current]

render(<IterationsList current={current} iterations={iterations} />)

expect(
screen.getByRole('button', { name: 'Go to next iteration' })
).toBeDisabled()
})

test('previous iteration button is disabled when on first iteration', async () => {
const current = { idx: 1 }
const iterations = [current, { idx: 2 }]

render(<IterationsList current={current} iterations={iterations} />)

expect(
screen.getByRole('button', { name: 'Go to previous iteration' })
).toBeDisabled()
})