feat(editor): better handling of urls in constraints

geonetwork · Nov 14, 2024 · a944d3f · a944d3f
1 parent 6db6735
commit a944d3f
Show file tree

Hide file tree

Showing 6 changed files with 67 additions and 19 deletions.
diff --git a/libs/feature/editor/src/lib/components/constraint-card/constraint-card.component.html b/libs/feature/editor/src/lib/components/constraint-card/constraint-card.component.html
@@ -3,13 +3,13 @@
     class="mt-[-8px]"
     [label]="label"
     [hint]="hint"
-    [value]="constraint.text ?? ''"
+    [value]="constraint_.text ?? ''"
     (valueChange)="handleConstraintTextChange($event)"
   ></gn-ui-form-field-rich>
 
-  <div *ngIf="!showUrlInput" class="flex-none mt-1">
+  <div *ngIf="!showUrl" class="flex-none mt-1">
     <gn-ui-button
-      (buttonClick)="showUrlBtnClicked = true"
+      (buttonClick)="showUrl = true"
       type="gray"
       data-cy="add-url-btn"
     >
@@ -18,9 +18,9 @@
     </gn-ui-button>
   </div>
   <gn-ui-url-input
-    *ngIf="showUrlInput"
+    *ngIf="showUrl"
     class="mt-3.5"
-    [value]="constraint.url"
+    [value]="constraint_.url?.toString()"
     (valueChange)="handleURLChange($event)"
     [showUploadButton]="false"
   >

diff --git a/libs/feature/editor/src/lib/components/constraint-card/constraint-card.component.spec.ts b/libs/feature/editor/src/lib/components/constraint-card/constraint-card.component.spec.ts
@@ -27,15 +27,18 @@ describe('ConstraintCardComponent', () => {
 
   describe('showUrlInput', () => {
     it('returns true if url is not nullish', () => {
-      component.constraint.url = new URL('https://example.com/my-license.pdf')
-      expect(component.showUrlInput).toBe(true)
+      component.constraint = {
+        text: 'abcd',
+        url: new URL('https://example.com/my-license.pdf'),
+      }
+      expect(component.showUrl).toBe(true)
     })
     it('returns true if showUrl button was clicked once', () => {
-      component.showUrlBtnClicked = true
-      expect(component.showUrlInput).toBe(true)
+      component.showUrl = true
+      expect(component.showUrl).toBe(true)
     })
     it('returns false otherwise', () => {
-      expect(component.showUrlInput).toBe(false)
+      expect(component.showUrl).toBe(false)
     })
   })
 })
diff --git a/libs/feature/editor/src/lib/components/constraint-card/constraint-card.component.ts b/libs/feature/editor/src/lib/components/constraint-card/constraint-card.component.ts
@@ -44,25 +44,26 @@ import { iconoirPlus } from '@ng-icons/iconoir'
 })
 export class ConstraintCardComponent {
   @Input() label: string
-  @Input() constraint: Constraint
+  constraint_: Constraint
+  @Input() set constraint(v: Constraint) {
+    this.constraint_ = v
+    this.showUrl = this.showUrl || !!v.url
+  }
   @Output() constraintChange = new EventEmitter<Constraint>()
 
   hint = 'editor.record.form.constraint.markdown.placeholder' // TODO: get text and translate
-  showUrlBtnClicked = false
-  get showUrlInput() {
-    return this.showUrlBtnClicked || !!this.constraint.url?.toString()
-  }
+  showUrl = false
 
   handleConstraintTextChange(text: string) {
     this.constraintChange.emit({
-      ...this.constraint,
+      ...this.constraint_,
       text,
     })
   }
 
   handleURLChange(url: string | null) {
     this.constraintChange.emit({
-      text: this.constraint.text,
+      text: this.constraint_.text,
       ...(url && { url: new URL(url) }),
     })
   }

diff --git a/libs/ui/inputs/src/lib/url-input/url-input.component.html b/libs/ui/inputs/src/lib/url-input/url-input.component.html
@@ -4,7 +4,7 @@
     class="gn-ui-text-input px-[var(--text-padding)]"
     [ngClass]="extraClass"
     type="url"
-    [value]="value?.toString() ?? ''"
+    [value]="inputValue"
     (input)="handleInput($event)"
     (keydown.enter)="handleUpload(input)"
     [placeholder]="placeholder"

diff --git a/libs/ui/inputs/src/lib/url-input/url-input.component.spec.ts b/libs/ui/inputs/src/lib/url-input/url-input.component.spec.ts
@@ -120,24 +120,52 @@ describe('UrlInputComponent', () => {
       it('is disabled if parent set it as disabled', () => {
         component.disabled = true
         inputEl.value = ''
+        inputEl.dispatchEvent(new Event('input'))
         fixture.detectChanges()
         expect(button.componentInstance.disabled).toBe(true)
       })
       it('is disabled if value is empty', () => {
         inputEl.value = ''
+        inputEl.dispatchEvent(new Event('input'))
         fixture.detectChanges()
         expect(button.componentInstance.disabled).toBe(true)
       })
       it('is disabled if value is not an URL', () => {
         inputEl.value = 'hello'
+        inputEl.dispatchEvent(new Event('input'))
         fixture.detectChanges()
         expect(button.componentInstance.disabled).toBe(true)
       })
       it('is not disabled otherwise', () => {
         inputEl.value = 'http://hello.org'
+        inputEl.dispatchEvent(new Event('input'))
         fixture.detectChanges()
         expect(button.componentInstance.disabled).toBeFalsy()
       })
     })
+
+    describe('input value', () => {
+      it('changes if the component input resolves to a different url', () => {
+        inputEl.value = 'http://aaa.com/1234'
+        inputEl.dispatchEvent(new Event('input'))
+        component.value = 'http://aaa.com/bcd'
+        fixture.detectChanges()
+        expect(inputEl.value).toEqual('http://aaa.com/bcd')
+      })
+      it('does not change if the component input is different that the current value but resolves to the same url', () => {
+        inputEl.value = 'http://aaa.com/1234 5678'
+        inputEl.dispatchEvent(new Event('input'))
+        component.value = 'http://aaa.com/1234%205678'
+        fixture.detectChanges()
+        expect(inputEl.value).toEqual('http://aaa.com/1234 5678')
+      })
+      it('does not change if both the component input and the current input are not valid urls', () => {
+        inputEl.value = 'blargz'
+        inputEl.dispatchEvent(new Event('input'))
+        component.value = undefined
+        fixture.detectChanges()
+        expect(inputEl.value).toEqual('blargz')
+      })
+    })
   })
 })
diff --git a/libs/ui/inputs/src/lib/url-input/url-input.component.ts b/libs/ui/inputs/src/lib/url-input/url-input.component.ts
@@ -1,4 +1,5 @@
 import {
+  ChangeDetectionStrategy,
   ChangeDetectorRef,
   Component,
   EventEmitter,
@@ -26,9 +27,21 @@ import { iconoirArrowUp, iconoirLink } from '@ng-icons/iconoir'
       size: '1.5em',
     }),
   ],
+  changeDetection: ChangeDetectionStrategy.OnPush,
 })
 export class UrlInputComponent {
-  @Input() value?: string
+  @Input() set value(v: string) {
+    // we're making sure to only update the input if the URL representation of it has changed; otherwise we keep it identical
+    // to avoid glitches when starting to write a URL and having some characters added/replaced automatically
+    if (!v || !this.isValidUrl(v)) return
+    if (
+      this.isValidUrl(this.inputValue) &&
+      new URL(v).toString() === new URL(this.inputValue).toString()
+    )
+      return
+    this.inputValue = v
+    this.cd.markForCheck()
+  }
   @Input() extraClass = ''
   @Input() placeholder = 'https://'
   @Input() disabled: boolean
@@ -40,10 +53,13 @@ export class UrlInputComponent {
   @Output() valueChange = new EventEmitter<string | null>()
   @Output() uploadClick = new EventEmitter<string>()
 
+  inputValue = ''
+
   constructor(private cd: ChangeDetectorRef) {}
 
   handleInput(event: Event) {
     const value = (event.target as HTMLInputElement).value
+    this.inputValue = value
     if (!value || !this.isValidUrl(value)) {
       this.valueChange.next(null)
       return