fix(ui-library): harmonised property names #981

JpunktWpunkt · 2024-03-05T15:27:49Z

thrbnhrtmnn

LGTM in Storybook, cant evaluate the other parts though

One small thing we could fix, while changing the index.stories.ts for the form caption group would be the default text of the default story messages from "hint" to "Hint-Message-Text" and from "error" to "Error-Message-Text".

Also we should fix the link to the Form Caption in the Intro. It broke when we got rid of the component grouping.

angsherpa456

LGTM

thrbnhrtmnn

Also LGTM, we can approve and merge once the conflict is solved

faselbaum · 2024-03-11T09:03:53Z

packages/ui-library/src/components/slider-two-values/range-legend-min-max-slider/index.test.ts

 import { BlrRangeLegendMinMaxSliderRenderFunction } from './renderFunction';
 import type { BlrRangeLegendMinMaxSliderType } from '.';

-import { expect, fixture } from "@open-wc/testing";
+import { expect, fixture } from '@open-wc/testing';
 import { querySelectorAllDeep, querySelectorDeep } from 'query-selector-shadow-dom';

 const sampleParams: BlrRangeLegendMinMaxSliderType = {
-    theme: 'Light',
-    rangeInputId: 'range-legend-cmpt',
-    startValue: '200$',
-    endValue: '400$',
-    size: 'lg',
-    list: [
-        '100$',
-        '200$',
-        '300$',
-        '400$',
-        '500$',
-    ],
-    stepFactor: 1,
-    btnVariant: 'primary',
-    incrementIcon: 'blrPlus',
-    decrementIcon: 'blrMinus',
-    onChange: () => null,
+  theme: 'Light',
+  rangeInputId: 'range-legend-cmpt',
+  startValue: '200$',
+  endValue: '400$',
+  size: 'lg',
+  list: ['100$', '200$', '300$', '400$', '500$'],
+  stepFactor: 1,
+  btnVariant: 'primary',
+  incrementIcon: 'blrPlus',
+  decrementIcon: 'blrMinus',
+  onChange: () => null,
 };

 describe('blr-range-legend-min-max-slider', () => {
-    it('is having two sliders containing the right className', async () => {
-        const element = await fixture(BlrRangeLegendMinMaxSliderRenderFunction(sampleParams));
-        const inputWrapper = querySelectorDeep('.range-wrapper', element.getRootNode() as HTMLElement);
-        const textareas = querySelectorAllDeep('input', inputWrapper?.getRootNode() as HTMLElement);
-        const classNameInputOne = textareas[0]?.className;
-        const classNameInputTwo = textareas[1]?.className;
-        
-       expect(classNameInputOne).to.contain('range'); 
-       expect(classNameInputTwo).to.contain('range'); 
-    });
+  it('is having two sliders containing the right className', async () => {
+    const element = await fixture(BlrRangeLegendMinMaxSliderRenderFunction(sampleParams));
+    const inputWrapper = querySelectorDeep('.range-wrapper', element.getRootNode() as HTMLElement);
+    const textareas = querySelectorAllDeep('input', inputWrapper?.getRootNode() as HTMLElement);
+    const classNameInputOne = textareas[0]?.className;
+    const classNameInputTwo = textareas[1]?.className;
+
+    expect(classNameInputOne).to.contain('range');
+    expect(classNameInputTwo).to.contain('range');
+  });

-    it('properly renders legend', async () => {
-        const element = await fixture(BlrRangeLegendMinMaxSliderRenderFunction(sampleParams));
-        const inputWrapper = querySelectorDeep('.range-wrapper', element.getRootNode() as HTMLElement);
-        const rangeNumbersElement = querySelectorDeep('.range__numbers', inputWrapper?.getRootNode() as HTMLElement);
-        const allRangePoints = querySelectorAllDeep('.range__point', rangeNumbersElement?.getRootNode() as HTMLParagraphElement);
+  it('properly renders legend', async () => {
+    const element = await fixture(BlrRangeLegendMinMaxSliderRenderFunction(sampleParams));
+    const inputWrapper = querySelectorDeep('.range-wrapper', element.getRootNode() as HTMLElement);
+    const rangeNumbersElement = querySelectorDeep('.range__numbers', inputWrapper?.getRootNode() as HTMLElement);
+    const allRangePoints = querySelectorAllDeep(
+      '.range__point',
+      rangeNumbersElement?.getRootNode() as HTMLParagraphElement
+    );

-        allRangePoints.forEach(point => {
-            expect(point).to.exist;
-        });
-        expect(allRangePoints.length).to.equal(sampleParams.list.length)
+    allRangePoints.forEach((point) => {
+      expect(point).to.exist;
    });
-})
+    expect(allRangePoints.length).to.equal(sampleParams.list.length);
+  });
+});


The formatting changes seem out of scope for this PR and could be removed.
What do you think? I'm curious, especially since i'm new to the team.
Should we just fix these things on the fly whenever we encounter them?

To be honest, I have no glue. I think this came when I solved the merge conflict from develop. I don't touch this file in this way.

packages/ui-library/src/components/form-caption-group/index.ts

…x/878-form-caption-group-harmonized-property-names

…perty-names

* fix(ui-library): harmonized property names * fix(storybook): changed values after review * fix(ui-library): harmonized naming in form-caption and dependencie files --------- Co-authored-by: David Kennedy <david.a.kennedy@accenture.com> Co-authored-by: Thorben Hartmann <122102805+thrbnhrtmnn@users.noreply.github.com>

fix(ui-library): harmonized property names

55cdf77

JpunktWpunkt linked an issue Mar 5, 2024 that may be closed by this pull request

Form Caption Group - Ensure Consistency between Storybook Documentation and Codebase Property Names #878

Closed

6 tasks

thrbnhrtmnn reviewed Mar 6, 2024

View reviewed changes

angsherpa456 previously approved these changes Mar 7, 2024

View reviewed changes

ChristianHoffmannS2 changed the title ~~fix(ui-library): harmonized property names~~ fix(ui-library): harmonised property names Mar 8, 2024

faselbaum added the 🦹 needs:reviewers (code) The issue or pull request needs additional code reviewers label Mar 8, 2024

fix(ui-library): merged current develop

859173a

JpunktWpunkt dismissed angsherpa456’s stale review via 859173a March 11, 2024 06:19

fix(storybook): changed values after review

a82d3c1

thrbnhrtmnn reviewed Mar 11, 2024

View reviewed changes

faselbaum self-requested a review March 11, 2024 08:54

faselbaum removed the 🦹 needs:reviewers (code) The issue or pull request needs additional code reviewers label Mar 11, 2024

fix(ui-library): solved mergeconflicts

a2ee77c

faselbaum reviewed Mar 11, 2024

View reviewed changes

faselbaum previously approved these changes Mar 11, 2024

View reviewed changes

faselbaum added 🦹 needs:reviewers (code) The issue or pull request needs additional code reviewers and removed 🦹 needs:reviewers (code) The issue or pull request needs additional code reviewers labels Mar 11, 2024

davidken91 reviewed Mar 11, 2024

View reviewed changes

packages/ui-library/src/components/form-caption-group/index.ts Show resolved Hide resolved

JpunktWpunkt added 2 commits March 12, 2024 13:46

fix(ui-library): merged current develop

3701db6

fix(ui-library): harmonized naming in form-caption and dependencie files

95e2648

JpunktWpunkt dismissed faselbaum’s stale review via 95e2648 March 12, 2024 13:07

Merge branch 'develop' of https://github.com/deven-org/boiler into fi…

b958436

…x/878-form-caption-group-harmonized-property-names

davidken91 approved these changes Mar 14, 2024

View reviewed changes

thrbnhrtmnn approved these changes Mar 14, 2024

View reviewed changes

Merge branch 'develop' into fix/878-form-caption-group-harmonized-pro…

25f7807

…perty-names

thrbnhrtmnn merged commit a54d6ab into develop Mar 14, 2024
4 of 5 checks passed

thrbnhrtmnn deleted the fix/878-form-caption-group-harmonized-property-names branch March 14, 2024 17:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(ui-library): harmonised property names #981

fix(ui-library): harmonised property names #981

JpunktWpunkt commented Mar 5, 2024

thrbnhrtmnn left a comment

angsherpa456 left a comment

thrbnhrtmnn left a comment

faselbaum Mar 11, 2024

JpunktWpunkt Mar 11, 2024

fix(ui-library): harmonised property names #981

fix(ui-library): harmonised property names #981

Conversation

JpunktWpunkt commented Mar 5, 2024

thrbnhrtmnn left a comment

Choose a reason for hiding this comment

angsherpa456 left a comment

Choose a reason for hiding this comment

thrbnhrtmnn left a comment

Choose a reason for hiding this comment

faselbaum Mar 11, 2024

Choose a reason for hiding this comment

JpunktWpunkt Mar 11, 2024

Choose a reason for hiding this comment