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

refactor(issues/KUI-1336): delete CoursePresentaion component and a small refactor in CourseMemo file #385

Merged
merged 2 commits into from
Jun 3, 2024

Conversation

amirhossein-haerian
Copy link
Contributor

This task (IF-1336 consists of two parts,

  1. Remove display of text and image in published course PM: for this part I realized that there is a component in the project that handles the display of the section that is meant to be deleted, the name of the component is "Course Presentation".
    I have deleted the component and all of the related parts that called it. I also needed to call jest --updateSnapshot to prevent tests to be failed.

  2. Also consider possibly removing code that customizes the printed/pdf version: for this part I have checked how the pdf will be created, I understood that inside the "CourseMemoLinks.js" there is a function for that which simply just calls "window.print()" to create a pdf version of the page. If we want to manage that what should not be in the pdf version of the page we need to use css styles. In our project we have a class called "d-print-none" which mange this and the "Course Presentation" had this className as well. So if we decide to add the text and imge to the PDF later we can simply do it by removing the "d-print-none".

<p className="mb-4">
{sectionsLabels.asterisk} {seasonStr(i18n.messages[memoLanguageIndex].extraInfo, validFromTerm)}
{sectionsLabels.asterisk} {seasonStr(extraInfo, validFromTerm)}
</p>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I realised that we got the data for "extraInfo" in previous steps so I decided to make this a little shorter in case of having cleaner code

@amirhossein-haerian amirhossein-haerian requested review from a user, belanglos and axelbjo June 3, 2024 09:28
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Code itself looks good!

But it would be great if you can change in type of commit message to "KUI-1336" (instead of "IF-1336") before merging

@amirhossein-haerian amirhossein-haerian changed the title refactor(issues/IF-1336): delete CoursePresentaion component and a small refactor in CourseMemo file refactor(issues/KUI-1336-1336): delete CoursePresentaion component and a small refactor in CourseMemo file Jun 3, 2024
@amirhossein-haerian amirhossein-haerian changed the title refactor(issues/KUI-1336-1336): delete CoursePresentaion component and a small refactor in CourseMemo file refactor(issues/KUI-1336): delete CoursePresentaion component and a small refactor in CourseMemo file Jun 3, 2024
@amirhossein-haerian
Copy link
Contributor Author

Code itself looks good!

But it would be great if you can change in type of commit message to "KUI-1336" (instead of "IF-1336") before merging

Sure, I will do that.

Copy link
Contributor

@belanglos belanglos left a comment

Choose a reason for hiding this comment

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

Looks good to me (LGTM)!

Let me now if you want to do look together at renaming branch and commit from IF to KUI!

@amirhossein-haerian amirhossein-haerian merged commit 3215364 into master Jun 3, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants