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

feat(inputs): detect constraint validation attribute changes #930

Merged

Conversation

blm768
Copy link
Contributor

@blm768 blm768 commented May 21, 2024

When attributes related to HTML constraint validation change on an input, they may change or even remove the validation error on the input. Ideally, we'd reflect the change of validation error in the enclosing field.

Proposed Changes

Changes to the following attributes on an invalid form element should trigger a call to checkHtml5Validity:

  • required
  • pattern
  • maxlength
  • minlength
  • max
  • min
  • step

Additionally, changes to the disabled attribute on the input or any ancestor <fieldset> should trigger a call to checkHtml5Validity.

Sample

<script setup lang="ts">
import { OCheckbox, OInput } from '@oruga-ui/oruga-next';
import { ref } from 'vue';

const val = ref("");
const required = ref(true);
const disabled = ref(false);
const groupDisabled = ref(false);
</script>

<template>
  <main>
    <OField label="Value">
      <fieldset :disabled="groupDisabled">
        <OInput v-model="val" :required :disabled />
      </fieldset>
    </OField>
    <OField>
      <OCheckbox v-model="required">Required</OCheckbox>
    </OField>
    <OField>
      <OCheckbox v-model="disabled">Disabled (directly)</OCheckbox>
    </OField>
    <OField>
      <OCheckbox v-model="groupDisabled">Disabled (group)</OCheckbox>
    </OField>
  </main>
</template>

Before

before.webm

After

after.webm

Copy link

netlify bot commented May 21, 2024

Deploy Preview for oruga-documentation-preview ready!

Name Link
🔨 Latest commit e17a4fa
🔍 Latest deploy log https://app.netlify.com/sites/oruga-documentation-preview/deploys/665f46d1e26f1c0008eba6c7
😎 Deploy Preview https://deploy-preview-930--oruga-documentation-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

codecov bot commented May 21, 2024

Codecov Report

Attention: Patch coverage is 61.29032% with 12 lines in your changes missing coverage. Please review.

Project coverage is 18.08%. Comparing base (484cfe8) to head (e17a4fa).
Report is 72 commits behind head on develop.

Files Patch % Lines
packages/oruga/src/composables/useInputHandler.ts 61.29% 8 Missing and 4 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##           develop     #930       +/-   ##
============================================
- Coverage    56.71%   18.08%   -38.64%     
============================================
  Files           30      292      +262     
  Lines         1511     7345     +5834     
  Branches       544     2135     +1591     
============================================
+ Hits           857     1328      +471     
- Misses         654     5067     +4413     
- Partials         0      950      +950     
Flag Coverage Δ
oruga-next 18.08% <61.29%> (-38.64%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@blm768
Copy link
Contributor Author

blm768 commented May 21, 2024

Without in-browser testing of some kind, it's hard to really automate testing of this change. Are there any plans to restore Cypress (or use another in-browser test framework) that require a full browser implementation of the DOM?

@mlmoravek mlmoravek changed the title Detect constraint validation attribute changes feat(inputs): detect constraint validation attribute changes Jun 3, 2024
@mlmoravek mlmoravek added the enhancement Improvements to existing features and functionality label Jun 3, 2024
@mlmoravek
Copy link
Member

@blm768 I really appreciate your clear and well written PRs and Issues. You have some very nice requests for improvements for Oruga!

@blm768
Copy link
Contributor Author

blm768 commented Jun 3, 2024

@blm768 I really appreciate your clear and well written PRs and Issues. You have some very nice requests for improvements for Oruga!

Thanks! Oruga has been providing significant value for me and the rest of the team I work with, so it's only fair that I contribute back, especially when it makes Oruga more suitable for my own use cases.

@mlmoravek
Copy link
Member

Without in-browser testing of some kind, it's hard to really automate testing of this change. Are there any plans to restore Cypress (or use another in-browser test framework) that require a full browser implementation of the DOM?

At the moment there are no plans to restore Cypress until most of the components have been unit tested. I think we have enough open stuff to do with the current stack until we can look further into more testing techniques.

@mlmoravek
Copy link
Member

Thanks! Oruga has been providing significant value for me and the rest of the team I work with, so it's only fair that I contribute back, especially when it makes Oruga more suitable for my own use cases.

This is how I became a maintainer for oruga :D

@blm768
Copy link
Contributor Author

blm768 commented Jun 4, 2024

At the moment there are no plans to restore Cypress until most of the components have been unit tested. I think we have enough open stuff to do with the current stack until we can look further into more testing techniques.

That's fair enough; there's certainly plenty of room for expanding the current tests.

@mlmoravek mlmoravek merged commit 4de3bd7 into oruga-ui:develop Jun 5, 2024
10 of 11 checks passed
@blm768 blm768 deleted the detect-validation-attribute-changes branch June 10, 2024 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvements to existing features and functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants