Observed in 3.3.6 as well so it is not a 3.3.7 regression.

Hi, I was looking forward to resolve this issue, but I have a question in order to find the best solution: if the ‘Review Lowest Scored Skill’ button does not show up, is the My Dashboard supposed to still stay in the middle of the screen, or eitherway it should be alligned to the right side? I think I know how to resolve the issue on both cases so if possible I would like to give it a go :)

Great question -- for simplicity, I would suggest always right-aligning that button. Thanks for checking!

Great question -- for simplicity, I would suggest always right-aligning that button. Thanks for checking!

Oh okay, I actually was already solving the issue having in mind that only the ‘Review Lowest Scored Skill’ button would make the My Dashboard button go to the right, but I can make it so that it always happens :). Can I be assigned to this issue?

@LKMartin12 Per the guidance at https://github.com/oppia/oppia/wiki/Contributing-code-to-Oppia#choosing-a-good-first-issue, please provide an explanation of what your PR will do (with names of files you're changing, what you plan to change in each file, etc.). If it looks good, we can assign you to this issue.

Please also follow the other instructions on that wiki page if you have not yet done so. Thanks!

@LKMartin12 Per the guidance at https://github.com/oppia/oppia/wiki/Contributing-code-to-Oppia#choosing-a-good-first-issue, please provide an explanation of what your PR will do (with names of files you're changing, what you plan to change in each file, etc.). If it looks good, we can assign you to this issue.

Please also follow the other instructions on that wiki page if you have not yet done so. Thanks!

Thank You for your guidance and pacience on guiding me! Could you just help me with one thing, I don't know who I should @ in my proposal for this issue, is it you @seanlip or is it someone else? Thank You for your time!

I was trying to run the code by using the command python -m scripts.start in my computer, it opens the page, I register but then there are no lessons like the fractions one shown as an example in the description of this issue, how do I make it so that I can get those database templates to experiment the changes in the code @Akhilesh-max ? Sorry for the inconvenience and thank you for your time!

Hello @LKMartin12, you’ll need to load dummy data. You can do this by navigating to the activity tabs on the admin page, or directly by going to /admin (http://localhost:8181/admin). Please ensure your server is running before you proceed.

While loading most of the dummy data, it’s important that you have the ‘curriculum admin’ role. You can assign this role to yourself from the roles section on the admin page, or directly by navigating to /admin#/roles. Hope this helps! Thanks..

@Akhilesh-max thank you for your assistance in helping me test the code, but I am running into an issue. When I load the dummy data (I generated and loaded everything possible, even tho it might not be needed for this particular test) and it created the lessons, but there was no content in them, they were blank, is there no dummy data that makes it so I can test just how you have here https://www.oppiatestserver.org/learn/math/fractions/practice/session?selected_subtopic_ids=%5B1%5D? If not, how can I make it so that I have a lesson with a practise test inside it, could you guide me?

Hi, @LKMartin12, please assign role of Topic Manager to yourself from the admin page. Once you’ve done that, you should populate the stories and subsequently add chapters to these stories using the Topic and Skill dashboard. This should help, I guess.

Hi, @LKMartin12, please assign role of Topic Manager to yourself from the admin page. Once you’ve done that, you should populate the stories and subsequently add chapters to these stories using the Topic and Skill dashboard. This should help, I guess.

Thank You @Akhilesh-max , it was in fact that role I was missing to be able to test the code!

As I was resolving the issue, there was one time I ran the code, and the buttons came out as in the foto which, was not the expected outcome ( and I already figured out why it turned that way ) but thought it was a nice positioning for the buttons so I wanted to show the result before I fix it, what is your opinion on them like this? Thank You for Your Attention!

@LKMartin12, I’m not sure if this is the intended appearance. Perhaps @seanlip could help us in decide whether the expected behavior described in the issue or the one you’re showing is correct. Could you also include the URL (localhost:8181/) in your browser’s URL bar in your screenshot?
Additionally, please ensure that your fix is effective in both scenarios - when the ‘review lowest scored skill’ button is present and when it's not.

Thank you.

No, I suggest that we keep to the original expected design please (see my previous comments). The proposed one has too many points of focus and is harder to make properly responsive. Thanks!

Bug Fix

So by analysing the code and checking the browser inspect tab, I was able to verify that, two footers were being created, one for the ‘Review Lowest Scored Skill’ and "My Dashboard" buttons and another one for the "Retry" button. But, when I would inspect the first footer, the code was leaving space for three buttons to be added, meaning that all three buttons were expected in the first footer, which is not the case. This would make that the property justify-content: space-between (which gets the free space between all buttons and makes sure that space is proportionally added between them) to divide the space accounting that it would be button - space - button - space - button, when it should only be button - space - button

With further analysing I found out this was caused in this piece of the HTML in the question-player.component.html

where we were iterating through all buttons in the resultActionButtons list in the question-player.component.ts, meaning that even in the first footer, space was being left for the 'Retry' button, which would never appear. To fix this I added a method in the TypeScript called firstFooterButtons(), which will create an auxiliary list with the first two elements of the resultActionButtons list, as they will always end up being the ‘Review Lowest Scored Skill’ and the "My Dashboard" button, and then in the HTML, instead of iterating through resultActionButtons, the code will iterate through the list returned by firstFooterButtons()

@Akhilesh-max , I am sorry to bother you again, but I wanted to ask if you knew how to fix an issue I encountered when trying to push my solution to my new branch in my fork.

In the TypeScript file I changed to make my solution, the terminal says that it has not been completly tested, probably due to the method I created there, but how do I fix this?

@LKMartin12 It appears that you’re modifying a TypeScript file. Whenever we alter a .ts file, it’s important to ensure that we’ve written corresponding frontend tests for any untested parts (it could be the part you have added or modified) of the question-player.component.ts file. Otherwise, the frontend tests might fail.

Please try writing the necessary frontend tests for this file. This wiki may help you in writing frontend tests.

Additionally, there's nothing like bothering as such. We’re part of a learning community. So, no need to hesitate in asking questions. However, it’s recommended to do some research beforehand.

@Akhilesh-max Thank You for your kindness, as it is my first time contributing to the project I just want to show that I am capable I guess ahah, and I do believe in my solution so I want to get it right. I read the wiki, and searched for the file where the tests of question-player.component.ts are made ( I assume that it is in question-player.component.spec.ts, correct me if I am wrong), but by analysing the file I could not comprehend what am I supposed to implement as a test, or if it is even needed for the kind of method I implemented, given that I think there are methods like this one in question-player.component.ts

that are just used as auxiliary methods, and from what I could find there is no test about it, just like my method is only an auxiliary method to return a list that will be iterated in the HTML. Could you give me your opinion about it? Do you have any idea of what I should implement? What should I do?

@LKMartin12, indeed, the tests for a file are typically located in a file with the same name but appended with spec.ts.

Additionally, It’s not that auxiliary functions don’t require tests at all. Whether they need tests primarily depends on the context and the complexity of the function. For instance, the getTotalQuestions() function you mentioned didn’t require tests because it’s a straightforward method that simply returns a static value, which is the total number of questions. There’s little complexity or risk of error in such a direct return statement. However, the firstFooterButtons() function that you have created does require a test because it involves more complex logic. It creates a new list based on a subset of an existing array, and this process could be prone to errors if not implemented correctly. Try writing tests that ensures that the function consistently returns the correct two buttons, especially if the contents of the resultActionButtons list change over time. It’s important to verify that the function behaves as expected under various conditions, which is why a test would be necessary.

Bug Fix Enhancement

As I was trying to push my previous solution to my branch in the fork I created I noticed my previous implementation would not pass in all the frontend tests, as the method I was using previously would not be tested as it was only called in the HTML. So I got to work on a solution that wouldn't require an additional function in the TypeScript file, and I was able to get the job done only by rework the HTML. The solution is the same as before, the only modification is that I now use slice to make it so that the for only iterates through the elements in the position 0 and 1 of the given array

Both when the ‘Review Lowest Scored Skill’ buttons appears and not the positioning of the 'My Dashboard' button is now the intended :) @seanlip

@LKMartin12 Thanks. The UI looks fine ... I'm not totally sure about the approach of slicing the buttons array programmatically, though, or even having a buttons array at all. If you can handle each of the buttons separately (perhaps using ng-if to decide when to display the "review lowest scored skill" one), that would probably be better.

Feel free to make a PR based on the above changes. Thanks!

@LKMartin12 Thanks. The UI looks fine ... I'm not totally sure about the approach of slicing the buttons array programmatically, though, or even having a buttons array at all. If you can handle each of the buttons separately (perhaps using ng-if to decide when to display the "review lowest scored skill" one), that would probably be better.

Feel free to make a PR based on the above changes. Thanks!

But that is what the line

""<div *ngIf="!(actionButton.type === 'RETRY_SESSION') && actionButton.type === 'REVIEW_LOWEST_SCORED_SKILL' && !(getWorstSkillIds().length === 0)""

is doing, and the buttons array wasn't something I implemented, it was part of the previous implementation of this part of the code which I tried to reuse so that I would have to change the least amount of code possible in order to keep it simple for future coders :) . Thank You for your kindness I will get the PR going as soon as I can!

@LKMartin12 Ah, I see. Hm, I think separate buttons is better, but I think changing the minimal amount of code for this is also fine, so feel free to go ahead. Thanks!

@seanlip just one question, so that the issue wouldn't be closed with my commit to my branch I didn't name the commit with "Fix Hashtag Issue Number", should I change my commit message to that before opening the pr? Thanks!

Yes. Follow the instructions in the PR guide. Thanks.