Skip to content
This repository has been archived by the owner on Jan 3, 2024. It is now read-only.

OcTextInput: add defaultValue prop and make clear button null the value #1636

Merged
merged 9 commits into from
Nov 3, 2021

Conversation

dschmidt
Copy link
Member

@dschmidt dschmidt commented Sep 5, 2021

Description

  1. Fixed the clear button being visible even when element is disabled
  2. Made clear button emit null instead of empty string
  3. Made input/change event handling a bit more consistent
  4. Added defaultValue prop that is for now passed as placeholder to the input field.

Needs UX concept by @kulmann and @tbsbdr

Related Issue

Motivation and Context

Same as #1634 and #1635

Instead of using native placeholder prop on the input element, I would propose to introduce the defaultValue property which for now uses placeholder but keeps that as implementation detail. That way I can already use the functionality in ownBrander 2.0 and will automatically benefit from a better implementation at some point in the future.

How Has This Been Tested?

Screenshots (if appropriate):

Only visual change: it's now possible to have an empty input field with clear button.
That is useful when a user overrides the default value with an empty string.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
    we don't emit empty strings anymore on clear, but afaict clearButtonEnabled is used nowhere, so this shouldn't break anything in ownCloud
  • Technical debt
  • Tests

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation added/updated

Open tasks:

  • ...

@sonarcloud
Copy link

sonarcloud bot commented Sep 5, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

87.5% 87.5% Coverage
0.0% 0.0% Duplication

@pascalwengerter pascalwengerter force-pushed the work/textinput_defaultValue branch from 6e43b0e to f36e43e Compare November 3, 2021 09:58
Copy link
Contributor

@pascalwengerter pascalwengerter left a comment

Choose a reason for hiding this comment

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

LGTM and thanks for taking care, would like to get a final approval from @kulmann before merging

@sonarcloud
Copy link

sonarcloud bot commented Nov 3, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

86.7% 86.7% Coverage
0.0% 0.0% Duplication

Copy link
Member

@kulmann kulmann left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@kulmann kulmann merged commit b1c5ae8 into master Nov 3, 2021
@delete-merged-branch delete-merged-branch bot deleted the work/textinput_defaultValue branch November 3, 2021 13:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants