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

Refactor contest navigation to use navigable contests and add slugPrefix getter. #2363

Merged
merged 6 commits into from
Oct 25, 2024
Merged

Refactor contest navigation to use navigable contests and add slugPrefix getter. #2363

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1b8cbee
Refactor contest navigation to use navigable contests and add slugPre…
rohitpaulk Oct 25, 2024
d6ce5a9
Refactor nextContest and previousContest getters for cleaner logic.
rohitpaulk Oct 25, 2024
19ed47f
improve leaderboard entry colours
rohitpaulk Oct 25, 2024
1f1355f
update green bg
rohitpaulk Oct 25, 2024
b43ddad
Update contests-test.js to improve next button assertions and navigat…
rohitpaulk Oct 25, 2024
5131493
update nav
rohitpaulk Oct 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/components/contest-page/leaderboard-entry.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<a
href="/users/{{@entry.user.username}}"
target="_blank"
class="rounded-md w-full border {{this.colorClasses}} dark:hover:bg-gray-800 p-2.5 flex items-center justify-between group/leaderboard-entry"
class="rounded-md w-full border {{this.colorClasses}} dark:hover:bg-gray-700/40 p-2.5 flex items-center justify-between group/leaderboard-entry"
...attributes
rel="noopener noreferrer"
>
Expand Down
4 changes: 2 additions & 2 deletions app/components/contest-page/leaderboard-entry.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ export default class ContestPageLeaderboardEntryComponent extends Component<Sign
if (this.args.entry.isBanned) {
return 'dark:border-gray-500 opacity-25 grayscale';
} else {
return 'dark:border-green-500';
return 'dark:border-green-500/70 dark:bg-green-500/5';
}
} else {
return 'dark:border-white/10 dark:hover:border-gray-700';
return 'dark:border-white/10 dark:hover:border-white/30';
}
}

Expand Down
6 changes: 3 additions & 3 deletions app/components/contest-page/navigation.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@
@route="contest"
@model={{this.previousContest.slug}}
@disabled={{this.isPreviousContestDisabled}}
class="flex items-center rounded-l-md border border-r-[0.5px] border-gray-800 hover:border-gray-700 hover:bg-gray-800 px-2 py-1.5
class="flex items-center rounded-l-md border border-r-[0.5px] border-white/15 hover:border-white/20 hover:bg-white/5 pl-1.5 pr-2 py-1.5
{{if this.isPreviousContestDisabled 'pointer-events-none opacity-50'}}"
data-test-previous-contest-button
>
{{svg-jar "chevron-left" class="w-4 fill-current"}}
Previous
Prev
</LinkTo>
</div>

Expand All @@ -18,7 +18,7 @@
@route="contest"
@model={{this.nextContest.slug}}
@disabled={{this.isNextContestDisabled}}
class="flex items-center rounded-r-md border border-l-[0.5px] border-gray-800 hover:border-gray-700 hover:bg-gray-800 px-2 py-1.5
class="flex items-center rounded-r-md border border-l-[0.5px] border-white/15 hover:border-white/20 hover:bg-white/5 pl-2 pr-1.5 py-1.5
{{if this.isNextContestDisabled 'pointer-events-none opacity-50'}}"
data-test-next-contest-button
>
Expand Down
25 changes: 9 additions & 16 deletions app/components/contest-page/navigation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export default class ContestPageNavigationComponent extends Component<Signature>
@service declare date: DateService;

get currentContestIndex(): number {
return this.sortedContests.indexOf(this.args.contest);
return this.sortedNavigableContests.indexOf(this.args.contest);
}

get isNextContestDisabled(): boolean {
Expand All @@ -27,31 +27,24 @@ export default class ContestPageNavigationComponent extends Component<Signature>
return !this.previousContest;
}

get navigableContests(): ContestModel[] {
return this.args.allContests.filter((contest) => contest.slugPrefix === this.args.contest.slugPrefix);
}

get nextContest(): ContestModel | null {
if (this.currentContestIndex < this.sortedContests.length - 1) {
const nextContest = this.sortedContests[this.currentContestIndex + 1] as ContestModel;
const oneWeekFromNow = new Date(this.date.now() + 7 * 24 * 60 * 60 * 1000);

if (nextContest.startsAt < oneWeekFromNow) {
return nextContest;
} else {
return null;
}
} else {
return null;
}
return this.sortedNavigableContests[this.currentContestIndex + 1] || null;
}

get previousContest(): ContestModel | null {
if (this.currentContestIndex > 0) {
return this.sortedContests[this.currentContestIndex - 1] as ContestModel;
return this.sortedNavigableContests[this.currentContestIndex - 1] || null;
} else {
return null;
}
}

get sortedContests(): ContestModel[] {
return this.args.allContests.sortBy('startsAt');
get sortedNavigableContests(): ContestModel[] {
return this.navigableContests.sortBy('startsAt');
}
}

Expand Down
4 changes: 4 additions & 0 deletions app/models/contest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,8 @@ export default class ContestModel extends Model {
get leaderboardEntriesRevealedAt(): Date {
return new Date(this.startsAt.getTime() + 1000 * 60 * 60 * 24); // 24 hours after contest starts
}

get slugPrefix(): string {
return this.slug.replace(/-\d+$/, '');
}
}
17 changes: 14 additions & 3 deletions tests/acceptance/contests-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,13 +162,19 @@ module('Acceptance | contests-test', function (hooks) {
assert.strictEqual(currentURL(), '/contests/weekly-1', 'Previous button is disabled when there are no more previous contests');

await contestsPage.headerNavigation.clickOnNextContestButton();
await contestsPage.headerNavigation.clickOnNextContestButton();
assert.strictEqual(currentURL(), '/contests/weekly-2', 'Next button works');

await contestsPage.headerNavigation.clickOnNextContestButton();
assert.strictEqual(currentURL(), '/contests/weekly-3', 'Next button works');

await contestsPage.headerNavigation.clickOnNextContestButton();
assert.strictEqual(currentURL(), '/contests/weekly-4', 'Next button works');

await contestsPage.headerNavigation.clickOnNextContestButton();
assert.strictEqual(currentURL(), '/contests/weekly-5', 'Next button works');

assert.strictEqual(currentURL(), '/contests/weekly-3', 'Next button is disabled when the next contest is the second contest from present');
await contestsPage.headerNavigation.clickOnNextContestButton();
assert.strictEqual(currentURL(), '/contests/weekly-5', 'Next button is disabled when contest is last');
});

test('prize details navigation buttons work', async function (assert) {
Expand All @@ -192,7 +198,12 @@ module('Acceptance | contests-test', function (hooks) {
assert.strictEqual(currentURL(), '/contests/weekly-3', 'Next button works');

await contestsPage.prizeDetailsNavigation.clickOnNextContestButton();
assert.strictEqual(currentURL(), '/contests/weekly-4', 'Next button works');

await contestsPage.prizeDetailsNavigation.clickOnNextContestButton();
assert.strictEqual(currentURL(), '/contests/weekly-5', 'Next button works');

assert.strictEqual(currentURL(), '/contests/weekly-3', 'Next button is disabled when the next contest is the second contest from present');
await contestsPage.prizeDetailsNavigation.clickOnNextContestButton();
assert.strictEqual(currentURL(), '/contests/weekly-5', 'Next button is disabled when contest is last');
Comment on lines +201 to +207
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider refactoring to reduce test duplication.

While the assertions are thorough, there's significant duplication between the header and prize details navigation tests. Consider extracting common navigation test logic into a shared helper function.

Example refactor:

function assertNavigationBehavior(assert, navigationComponent, description) {
  await navigationComponent.clickOnNextContestButton();
  assert.strictEqual(currentURL(), '/contests/weekly-4', `${description} Next button works`);

  await navigationComponent.clickOnNextContestButton();
  assert.strictEqual(currentURL(), '/contests/weekly-5', `${description} Next button works`);

  await navigationComponent.clickOnNextContestButton();
  assert.strictEqual(currentURL(), '/contests/weekly-5', `${description} Next button is disabled when contest is last`);
}

// Usage in tests:
test('header navigation buttons work', async function(assert) {
  // ... existing setup ...
  await assertNavigationBehavior(assert, contestsPage.headerNavigation, 'Header navigation:');
});

test('prize details navigation buttons work', async function(assert) {
  // ... existing setup ...
  await assertNavigationBehavior(assert, contestsPage.prizeDetailsNavigation, 'Prize details navigation:');
});

});
});
Loading