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

Fix shifting div from KTextbox #279

Merged
merged 4 commits into from
Nov 30, 2021
Merged

Conversation

sairina
Copy link
Contributor

@sairina sairina commented Nov 19, 2021

Description

Changes the v-if to the v-show in the KeenUI textbox so that the element is already in the DOM and we can toggle its display.

This is useful because it will keep that div below KTextbox for error validation, and prevent the UI from "shifting" whether or not there is an error validation for the field.

Issue addressed

Before/after screenshots

See learningequality/kolibri#8746

Steps to test

After linking KDS to Kolibri, see the following for testing: learningequality/kolibri#8746

Does this introduce any tech-debt items?

  • We will want to double check that the places in Kolibri that this affects (see: Fix KTextbox in form vertical shift kolibri#8746) are the only places where the shifting needs to be fixed.
  • How can we document this, if this is how we will continue to deal with this issue in this way? Or, what can be done to be a more permanent fix?

Testing checklist

  • Contributor has fully tested the PR manually
  • If there are any front-end changes, before/after screenshots are included

Reviewer guidance

  • Is the code clean and well-commented (commented via git commit)

Post-merge updates and tracking

  • Learning Platform update PR is submitted
  • Learning Platform update PR is merged

Comments

@sairina sairina marked this pull request as ready for review November 19, 2021 05:44
Copy link
Contributor

@indirectlylit indirectlylit left a comment

Choose a reason for hiding this comment

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

possibly a simpler fix?

lib/keen/UiTextbox.vue Outdated Show resolved Hide resolved
Co-authored-by: Devon Rueckner <[email protected]>
@sairina
Copy link
Contributor Author

sairina commented Nov 24, 2021

@indirectlylit That was definitely the simpler fix. Seems to fix the issues that were found in learningequality/kolibri#8675

Copy link
Contributor

@indirectlylit indirectlylit left a comment

Choose a reason for hiding this comment

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

cool, thank you.

before merging, would you mind adding an entry to the changelog?

@indirectlylit
Copy link
Contributor

Darn, I forgot about this one before I tagged 1.2.0! I've pushed updates and will make a 1.2.1 now.

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.

2 participants