Fixed Update #8532 Replace TextTruncator with TextTruncatorCss #10518

Pursottam6003 · 2023-04-19T14:36:10Z

Summary

From the Issue #8532 I have changed the TextTruncator with TextTruncatorCss in two files only ie index.vue and AlsoInThis.vue

References

From the Issue discussion, i got the little clue to do this #8532

Reviewer guidance

I have been asked to do changes in step by step so that it would be easy to be evaluated and make corrections

Testing checklist

Contributor has fully tested the PR manually
If there are any front-end changes, before/after screenshots are included
Critical user journeys are covered by Gherkin stories
Critical and brittle code paths are covered by unit tests

PR process

PR has the correct target branch and milestone
PR has 'needs review' or 'work-in-progress' label
If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
If this is an important user-facing change, PR or related issue has a 'changelog' label
If this includes an internal dependency change, a link to the diff is provided

Reviewer checklist

Automated test coverage is satisfactory
PR is fully functional
PR has been tested for accessibility regressions
External dependency files were updated if necessary (yarn and pip)
Documentation is updated
Contributor is in AUTHORS.md

…f TextTruncator

github-actions · 2023-04-19T14:55:17Z

Build Artifacts

Asset type	Download link
PEX file	kolibri-0.16.0a12.dev0_git.247.gbd64aac5.pex
Windows Installer (EXE)	kolibri-0.16.0a12.dev0+git.247.gbd64aac5-unsigned.exe
Debian Package	kolibri_0.16.0a12.dev0+git.247.gbd64aac5-0ubuntu1_all.deb
Mac Installer (DMG)	kolibri-0.16.0a12.dev0+git.247.gbd64aac5-0.3.0.dmg
Source Tarball	kolibri-0.16.0a12.dev0+git.247.gbd64aac5.tar.gz
WHL file	kolibri-0.16.0a12.dev0+git.247.gbd64aac5-py2.py3-none-any.whl

Pursottam6003 · 2023-04-21T06:10:57Z

Hii @MisRob Could you please review this PR does it meet the requirements if not kindly let me know where should i need modifications

MisRob · 2023-04-21T06:58:53Z

Thank you, @Pursottam6003, I will review it. Thank you for proceeding in smaller chunks, this will also help me to follow up with you sooner.

MisRob · 2023-04-21T07:31:13Z

@Pursottam6003 In your upcoming pull requests, could you please add the reviewer guidance in a way that communicates how to preview places that were updated from a user point of view (whenever it's relevant)?

This would help a reviewer and especially our QA team as they will know where to navigate as a user and what places to check to confirm fixes, test for regressions, etc. (QA team can't determine where to go in the app according to file names). It doesn't need to be extremely detailed, just a couple of bullet points to give direction will do. Often it's similar to the way you previewed updates on your dev server so as you're working next time, you could just note it down and then paste it to your PR)

Pursottam6003 · 2023-04-21T07:42:58Z

Hii @MisRob Could you please review this PR does it meet the requirements if not kindly let me know where should i need modifications

Sure mam I will definitely add Reviewer Guidelines in upcomming PRs

MisRob

Thank you, @Pursottam6003. Some changes will be needed, please see the comment marked as blocking. Also, if you could reference places where you tested this as you're working on the update, that'd be helpful.

MisRob · 2023-04-26T10:34:59Z

.../plugins/coach/assets/src/views/plan/LessonResourceSelectionPage/LessonContentCard/index.vue

@@ -60,7 +60,7 @@
  import CoachContentLabel from 'kolibri.coreVue.components.CoachContentLabel';
  import ContentIcon from 'kolibri.coreVue.components.ContentIcon';
  import { validateLinkObject, validateContentNodeKind } from 'kolibri.utils.validators';
-  import TextTruncator from 'kolibri.coreVue.components.TextTruncator';
+  import TextTruncatorCss from 'kolibri.coreVue.components.TextTruncatorCss'; //update


[minor] //update comment can be cleaned up. Same applies to the other updated component.

MisRob · 2023-04-26T10:54:12Z

.../plugins/coach/assets/src/views/plan/LessonResourceSelectionPage/LessonContentCard/index.vue

@@ -35,7 +35,7 @@
      <div v-if="message" class="message" :style="{ color: $themeTokens.text }">
        {{ message }}
      </div>
-      <TextTruncator
+      <TextTruncatorCss
        v-if="!windowIsSmall"
        :text="description"
        :maxHeight="80"


[blocking] TextTruncatorCss doesn't have maxHeight property. Instead, it has maxLines which expects a number of lines instead of height information. As we're moving to TextTruncatorCss, these places in the code will need to be updated to its interface too. I'd recommend briefly skimming through TextTruncatorCss file to understand it. The same applies to the other updated component.

Sure mam, got it thanks for the review

…extTruncatorCss

Pursottam6003 · 2023-04-27T12:06:39Z

Thank you, @Pursottam6003. Some changes will be needed, please see the comment marked as blocking. Also, if you could reference places where you tested this as you're working on the update, that'd be helpful.

Hii @MisRob now I have resolved the invalid props reference error and tested the code both manually and by using the yarn run test and pre-commit run --all-files

Now the TextTruncatorCss is successfully working inside the kolibri/plugins/coach/assets/src/views/plan/LessonResourceSelectionPage/LessonContentCard/index.vue

<TextTruncatorCss
        v-if="!windowIsSmall"
        :text="description"
        :maxLines="3"
        class="description"
      />

We can clearly see since the maxLines is set to 3 and if it exceeds the 3 lines it will be truncated with ....

MisRob

Thank you for your contribution and continuous cooperation, @Pursottam6003. It's looking good and I can confirm it works as expected in the other component as well.

If you'd like to continue working on #8532, feel free to proceed. It should be more or less the same process - replacing the component, props, and then checking if it works as expected in places where updated components are used. As mentioned before if you could briefly note how to navigate to places you tested in your next PR description, it'd be welcome, so that we know what to test (no need to update screenshots for all of them unless there are some user-facing changes)

update index.vue and AlsoInThis.vue to use TextTruncatorCss instead o…

6214dcc

…f TextTruncator

github-actions bot added APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) APP: Learn Re: Learn App (content, quizzes, lessons, etc.) SIZE: small labels Apr 19, 2023

MisRob self-requested a review April 21, 2023 06:57

MisRob requested changes Apr 26, 2023

View reviewed changes

resolved the invalid props error and resolve the minor issues using T…

380327d

…extTruncatorCss

MisRob self-requested a review April 28, 2023 04:41

MisRob approved these changes Apr 28, 2023

View reviewed changes

MisRob mentioned this pull request Apr 28, 2023

Replace TextTruncator by TextTruncatorCss #8532

Closed

MisRob merged commit c477af0 into learningequality:develop Apr 28, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed Update #8532 Replace TextTruncator with TextTruncatorCss #10518

Fixed Update #8532 Replace TextTruncator with TextTruncatorCss #10518

Pursottam6003 commented Apr 19, 2023

github-actions bot commented Apr 19, 2023 •

edited

Loading

Pursottam6003 commented Apr 21, 2023

MisRob commented Apr 21, 2023

MisRob commented Apr 21, 2023

Pursottam6003 commented Apr 21, 2023

MisRob left a comment

MisRob Apr 26, 2023

Pursottam6003 Apr 27, 2023

MisRob Apr 26, 2023

Pursottam6003 Apr 27, 2023

Pursottam6003 commented Apr 27, 2023

MisRob left a comment

Fixed Update #8532 Replace TextTruncator with TextTruncatorCss #10518

Fixed Update #8532 Replace TextTruncator with TextTruncatorCss #10518

Conversation

Pursottam6003 commented Apr 19, 2023

Summary

References

Reviewer guidance

Testing checklist

PR process

Reviewer checklist

github-actions bot commented Apr 19, 2023 • edited Loading

Build Artifacts

Pursottam6003 commented Apr 21, 2023

MisRob commented Apr 21, 2023

MisRob commented Apr 21, 2023

Pursottam6003 commented Apr 21, 2023

MisRob left a comment

Choose a reason for hiding this comment

MisRob Apr 26, 2023

Choose a reason for hiding this comment

Pursottam6003 Apr 27, 2023

Choose a reason for hiding this comment

MisRob Apr 26, 2023

Choose a reason for hiding this comment

Pursottam6003 Apr 27, 2023

Choose a reason for hiding this comment

Pursottam6003 commented Apr 27, 2023

MisRob left a comment

Choose a reason for hiding this comment

github-actions bot commented Apr 19, 2023 •

edited

Loading