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

Wraps KRadioButton label text #380

Merged
merged 3 commits into from
Nov 3, 2022
Merged

Wraps KRadioButton label text #380

merged 3 commits into from
Nov 3, 2022

Conversation

akolson
Copy link
Member

@akolson akolson commented Oct 26, 2022 •
edited
Loading

Description

This pr wraps the KRadioButton label instead of truncating it. Also, a new prop truncateLabel has been added to truncate the text explicitly. Finally, the KRadioButton has been streamlined to match KCheckbox's API.

Issue addressed

Addresses #349 #350

Before/after screenshots

Steps to test

Please follow the steps to test in Kolibri's corresponding PR learningequality/kolibri#9802
Read the updated documentation page and preview KRadioButton for usage and examples

(optional) Implementation notes

At a high level, how did you implement this?

The KRadioButton still works the same way however its API has been made consistent with that of the kCheckbox as proposed in #349. Also to truncate text, one has to explicitly specify that they want to do so via the new truncateLabel prop

Does this introduce any tech-debt items?

Testing checklist

  • Contributor has fully tested the PR manually
  • If there are any front-end changes, before/after screenshots are included
  • Critical and brittle code paths are covered by unit tests
  • The change has been added to the changelog

Reviewer guidance

  • Is the code clean and well-commented?
  • Are there tests for this change?
  • Are all UI components LTR and RTL compliant (if applicable)?

Post-merge updates and tracking

  • Learning Platform update PR is submitted
  • Learning Platform update PR is merged
  • Studio update PR is submitted
  • Studio update PR is merged
  • Data Portal update PR is submitted
  • Data Portal update PR is merged

Comments

@akolson
aligns kradiobutton api with kcheckbox's. Changes label from truncate…
f083b25 
… to wrap. Adds truncateLabel prop to truncate text if necessary. updates documenation to reflect changes
@akolson
updates documentation
ba2006c
@akolson akolson mentioned this pull request Oct 26, 2022
Merged
9 tasks
@akolson akolson added the TODO: needs review Waiting for review label Oct 26, 2022
@akolson akolson marked this pull request as ready for review October 27, 2022 12:41
nucleogenesis
nucleogenesis approved these changes Oct 31, 2022
View reviewed changes
Copy link
Member

@nucleogenesis nucleogenesis left a comment

Choose a reason for hiding this comment

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

Changes look great. I don't think there are any breaking changes here as the motivation behind this change, in part, is that we don't want to truncate text in most (if not all) places so I appreciate retaining the previous behavior as prop just in case we find that there is a good use case for a truncated radio button label.

@akolson akolson requested a review from nucleogenesis November 1, 2022 17:19
@akolson akolson merged commit f85ddfb into learningequality:develop Nov 3, 2022
LianaHarris360 added a commit to LianaHarris360/kolibri-design-system that referenced this pull request Nov 16, 2022
@LianaHarris360
Squashed commit of the following:
3e936e1 
commit f85ddfb
Merge: 29122c4 9d29fe1
Author: akolson <[email protected]>
Date:   Thu Nov 3 22:42:28 2022 +0300

    Merge pull request learningequality#380 from akolson/standardize-kradiobutton-labels

    Wraps KRadioButton label text

commit 9d29fe1
Author: Samson Akol <[email protected]>
Date:   Wed Oct 26 23:34:47 2022 +0300

    updates changelog

commit ba2006c
Author: Samson Akol <[email protected]>
Date:   Wed Oct 26 22:23:03 2022 +0300

    updates documentation

commit f083b25
Author: Samson Akol <[email protected]>
Date:   Wed Oct 26 22:04:02 2022 +0300

    aligns kradiobutton api with kcheckbox's. Changes label from truncate to wrap. Adds truncateLabel prop to truncate text if necessary. updates documenation to reflect changes

commit 29122c4
Merge: 115a19d 84ef450
Author: akolson <[email protected]>
Date:   Tue Oct 18 20:48:16 2022 +0300

    Merge pull request learningequality#377 from akolson/implement-kResponsiveWindow-composable

    Implement k responsive window composable

commit 84ef450
Author: Samson Akol <[email protected]>
Date:   Mon Oct 17 10:51:27 2022 +0300

    Adds more review feedback

commit 94abe9e
Author: Samson Akol <[email protected]>
Date:   Fri Oct 14 19:16:35 2022 +0300

    Improves test descriptions

commit 6dc1d50
Author: Samson Akol <[email protected]>
Date:   Fri Oct 14 18:18:08 2022 +0300

    Adds tests to useKWindowDimensions and useKResponsiveWindow

commit 51d80a3
Author: Samson Akol <[email protected]>
Date:   Thu Oct 13 13:25:31 2022 +0300

    minor string refactors

commit 794ac2a
Author: Samson Akol <[email protected]>
Date:   Thu Oct 13 12:55:00 2022 +0300

    Fixes responsiveness bug on IE

commit 445c740
Author: Samson Akol <[email protected]>
Date:   Wed Oct 12 23:01:34 2022 +0300

    Adds feedback from review

commit 68266f0
Author: Samson Akol <[email protected]>
Date:   Wed Oct 12 18:53:17 2022 +0300

    Improves jsdoc, fixes IE issue

commit b16c125
Author: Samson Akol <[email protected]>
Date:   Mon Oct 10 17:34:57 2022 +0300

    Adds use composition-api to composable due to @vue/composition-api shortcomings in vue2

commit 4075c57
Author: Samson Akol <[email protected]>
Date:   Mon Oct 10 13:30:48 2022 +0300

    Updates kresponsivewindow documentation

commit 5940c76
Author: Samson Akol <[email protected]>
Date:   Mon Oct 10 12:38:45 2022 +0300

    Adds feedback from code review

commit b0fc205
Author: Samson Akol <[email protected]>
Date:   Thu Oct 6 18:27:24 2022 +0300

    Updates changelog

commit e504a70
Author: Samson Akol <[email protected]>
Date:   Thu Oct 6 15:52:29 2022 +0300

    Code refactor

commit 76c13a8
Author: Samson Akol <[email protected]>
Date:   Thu Oct 6 15:12:43 2022 +0300

    Moves composition api initialization to useKResponsiveWindow

commit 9dd5975
Author: Samson Akol <[email protected]>
Date:   Wed Oct 5 17:52:57 2022 +0300

    Updates indentation

commit 078c000
Author: Samson Akol <[email protected]>
Date:   Wed Oct 5 16:34:14 2022 +0300

    uses anonymous function in documentation due to failures in character escaping

commit ad681dd
Author: Samson Akol <[email protected]>
Date:   Wed Oct 5 15:41:46 2022 +0300

    Configures composition api, and documents the useKResponsiveWindow

commit 2d1351e
Author: Samson Akol <[email protected]>
Date:   Tue Oct 4 17:45:50 2022 +0300

    Refactors code, adds documentation

commit 8fe1519
Author: Samson Akol <[email protected]>
Date:   Mon Oct 3 18:59:22 2022 +0300

    fixes bug in breakpoint media query listener

commit f6afdf6
Author: Samson Akol <[email protected]>
Date:   Mon Oct 3 18:56:47 2022 +0300

    Renames responsive window compodable to recommended composable file names

commit c4278c2
Author: Samson Akol <[email protected]>
Date:   Fri Sep 30 23:15:15 2022 +0300

    updates composable  name

commit fca8437
Author: Samson Akol <[email protected]>
Date:   Fri Sep 30 15:57:07 2022 +0300

    Fixes bugs in composable

commit e43b5a0
Author: Samson Akol <[email protected]>
Date:   Thu Sep 29 19:45:59 2022 +0300

    Port of KResponsiveWindowMixin to KResponsiveWindow composable
@marcellamaki marcellamaki mentioned this pull request Nov 22, 2022
Closed
@MisRob MisRob mentioned this pull request Sep 28, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marcellamaki marcellamaki Awaiting requested review from marcellamaki

@nucleogenesis nucleogenesis Awaiting requested review from nucleogenesis

Assignees
No one assigned
Labels
TODO: needs review Waiting for review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@akolson @nucleogenesis