Feature: Sortable Student Solutions #4315
Conversation
KevinMulhern
added
the
create-review-app
github-actions
bot
removed
the
create-review-app
KevinMulhern
force-pushed
the
feature/sorting-solutions
branch
from
b2b41d5 to
fb77174
Compare
KevinMulhern
force-pushed
the
feature/sorting-solutions
branch
from
fb77174 to
50cf074
Compare
KevinMulhern
force-pushed
the
feature/sorting-solutions
branch
from
50cf074 to
e262b06
Compare
KevinMulhern
force-pushed
the
feature/sorting-solutions
branch
from
e262b06 to
b84f48e
Compare
KevinMulhern
force-pushed
the
feature/sorting-solutions
branch
from
b84f48e to
4768e0c
Compare
KevinMulhern
force-pushed
the
feature/sorting-solutions
branch
from
4768e0c to
a40ecab
Compare
| @@ -52,6 +52,10 @@ Layout/EmptyLinesAroundAttributeAccessor: | |||
| Layout/SpaceAroundMethodCallOperator: | |||
| Enabled: true | |||
|
|
|||
| Layout/MultilineMethodCallIndentation: | |||
There was a problem hiding this comment.
Made this change because Rubocop was popping off about the project submissions query in the submissions controller.
I personally think the indented version is nicer/easer to read. But happy to revert this if that view isn't shared.
| def selected?(option) | ||
| return option[:default] if selected.compact.empty? | ||
|
|
||
| option[:value] == selected[:value] && option[:direction] == selected[:direction] |
There was a problem hiding this comment.
It's not great using a hash for this, I may do a follow up refactor to move this into a SortOption value object - this PR was getting too big.
| @@ -46,7 +46,7 @@ | |||
|
|
|||
| expect { course_builder } | |||
| .to change { existing_course.reload.title }.from('Development 101').to('Foundations') | |||
| .and change { existing_course.position }.from(2).to(1) | |||
| .and change { existing_course.position }.from(2).to(1) | |||
There was a problem hiding this comment.
Consequence of the Layout/MultilineMethodCallIndentation: Rubocop rule change
| FlagNotification.with(flag:, title: message.title, message: message.content, url: message.url) | ||
| .deliver_later(flag.project_submission.user) | ||
|
|
||
| FlagNotification |
There was a problem hiding this comment.
Consequence of the Layout/MultilineMethodCallIndentation: Rubocop rule change
KevinMulhern
force-pushed
the
feature/sorting-solutions
branch
from
a40ecab to
0d8ed5d
Compare
KevinMulhern
force-pushed
the
feature/sorting-solutions
branch
from
0d8ed5d to
3b171d6
Compare
KevinMulhern
force-pushed
the
feature/sorting-solutions
branch
from
3b171d6 to
797cb07
Compare
KevinMulhern
force-pushed
the
feature/sorting-solutions
branch
from
797cb07 to
609ed0e
Compare
KevinMulhern
force-pushed
the
feature/sorting-solutions
branch
from
609ed0e to
5939963
Compare
KevinMulhern
added
the
Status: Needs Review
KevinMulhern
force-pushed
the
feature/sorting-solutions
branch
from
5939963 to
a752126
Compare
|
|
||
| <div class="flex items-center mb-4 md:mb-0"> | ||
| <%= render ProjectSubmissions::LikeComponent.new(project_submission:, current_users_submission: current_users_submission?) %> | ||
| <%= render ProjectSubmissions::LikeComponent.new(project_submission:, current_users_submission: false) %> |
There was a problem hiding this comment.
Since the LikeComponent has a default value for current_users_submission do we need to specify that it's false or are you preferring to be explicit here?
There was a problem hiding this comment.
Nice spot, I don't mind this being implicit instead.
| <h3 class="mt-2 text-xl font-semibold text-gray-900 dark:text-gray-200">No solutions</h3> | ||
| <p class="mt-1 text-sm text-gray-500 dark:text-gray-400">Be the first</p> | ||
| <h3 class="mt-2 text-xl font-semibold text-gray-900 dark:text-gray-200">No solutions</h3> | ||
| <%= link_to 'Submit your solution', lesson_path(@lesson, anchor: 'project-solution'), class: 'mt-2 text-sm button button--secondary py-2', data: { turbo_frame: '_top'} %> |
There was a problem hiding this comment.
It feels a bit janky to navigate them back to the lesson page to submit their solution so they effectively have to click twice on the same text to submit their solution. Couldn't we use turbo's power here to just load the solution component in place? Maybe one for a future PR
There was a problem hiding this comment.
Good call out - yeah this was a bit of a cop out. The new user solution component being different from the item component we use on this page makes it a little bit more difficult to create a solution here. But I'll have a think about ways around that.
In the meantime I think we're probably better off prompting users to return to the project page instead of submitting. That feels a bit less janky.
| @@ -46,7 +46,7 @@ | |||
|
|
|||
| expect { course_builder } | |||
| .to change { existing_course.reload.title }.from('Development 101').to('Foundations') | |||
| .and change { existing_course.position }.from(2).to(1) | |||
| .and change { existing_course.position }.from(2).to(1) | |||
Because: - Being able to sort solutions will give users the ability to view solutions in different ways. Currently we only display solutions in order of most liked, which has resulted in the most liked solutions being all that a majority of our users ever see. This commit: - Adds a reusable sort component - Add the ability for users to sort solutions by newest, oldest and most liked - Includes redesigned UI for project and lesson pages - The user solution is now the only solution visible on project pages and moved into the lessons two column layout - The lesson complete/navigation buttons have also been moved into the lessons two column layout. - Adds a dedicated user solution component instead of using the project submission item component for both the current user and other user submissions. --- -
KevinMulhern
force-pushed
the
feature/sorting-solutions
branch
from
a752126 to
2a00fe0
Compare
Seems to be a bit of a performance issues with solution pages, probably missing an index. Reverting for now. Reverts #4315
#4222
Motivation
Currently, we only display a limited list of 10 solutions ordered by most liked on the project page. With no easy way to reorder the list, these 10 most liked solutions are all what most users will ever see and any new solutions have little chance of been seen. This significantly reduces the incentive of using this feature.
Being able to easily reorder the list in different ways, such as newest, oldest and most liked will allow users to view a much larger range of solutions and will greatly increase how often recent solutions are seen.
Screen.Recording.2024-01-13.at.15.55.55.mov
Redesigned Lesson UI
Our existing UI posed a few problems with sorting, mainly with how we display the current users solution. We "pin" it to the top of the list so its always in the same predictable place when the user goes looking for it.
For reference this is how it looks in production at the moment:
That doesn't work well with sorting. It looks odd if other items change position but the top item stays the same after changing the sort order. This problem necessitated a rethink of our solutions UI with the following user experience in mind:
A new "Your solution" component will be displayed within the lesson 2 column layout:
A dedicated component for the users project solution allows us to have nicer empty state which should makes it obvious if a solution has been submitted or not:
When viewing other learners solutions, we redirect users to the "All solutions" page for a individual projects. Using a full page instead of a limited view on the project itself will give us plenty of room to expand the feature set of this area in the future:
Screen.Recording.2024-01-13.at.15.58.01.mov
Bonus
With the solutions component moved within the two column layout, it made sense to move the complete and navigation buttons too. I think this is generally a better fit rather than having them off on their own “island” as per the old design.