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.

Sign up for GitHub

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

Jump to bottom

Read-only is not working in file-attachment #1200

Merged
merged 1 commit into from
Apr 30, 2024
Merged

Read-only is not working in file-attachment #1200

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 15 additions & 0 deletions ...content/jcr_root/content/forms/af/core-components-it/samples/fileinput/basic/.content.xml
View file
Open in desktop
Expand Up @@ -72,6 +72,21 @@
tooltip="This is short description"
multiSelection="true"
type="file"/>
<fileinput5
jcr:lastModified="{Date}2023-01-24T16:47:36.882+05:30"
jcr:lastModifiedBy="admin"
jcr:primaryType="nt:unstructured"
jcr:title="File Input - 5"
sling:resourceType="forms-components-examples/components/form/fileinput"
accept="[audio/*, video/*, image/*, text/*, application/pdf]"
buttonText="Attach Files"
enabled="{Boolean}true"
fieldType="file-input"
name="fileinput5"
readOnly="{Boolean}true"
textIsRich="[true,true]"
type="file[]"
visible="{Boolean}true"/>
<submit
jcr:lastModified="{Date}2023-01-17T16:28:58.844+05:30"
jcr:lastModifiedBy="admin"
Expand Down
1 change: 0 additions & 1 deletion ...fd/components/form/checkboxgroup/v1/checkboxgroup/clientlibs/site/js/checkboxgroupview.js
View file
Open in desktop
Expand Up @@ -156,7 +156,6 @@
}

updateEnabled(enabled, state) {
this.toggle(enabled, FormView.Constants.ARIA_DISABLED, true);
this.element.setAttribute(FormView.Constants.DATA_ATTRIBUTE_ENABLED, enabled);
let widgets = this.widget;
widgets.forEach(widget => {
Expand Down
1 change: 0 additions & 1 deletion ...root/apps/core/fd/components/form/dropdown/v1/dropdown/clientlibs/site/js/dropdownview.js
View file
Open in desktop
Expand Up @@ -84,7 +84,6 @@
}

updateEnabled(enabled, state) {
this.toggle(enabled, FormView.Constants.ARIA_DISABLED, true);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this code removed ? Isn't this backward incompatible

Copy link
Contributor Author

@pavi41 pavi41 Apr 25, 2024 •
edited

Choose a reason for hiding this comment

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

This line was adding aria-disabled attribute to the parent div which is of no use, so I had removed this line.

this.element.setAttribute(FormView.Constants.DATA_ATTRIBUTE_ENABLED, enabled);
let widget = this.widget;
if (enabled === false) {
Expand Down
14 changes: 14 additions & 0 deletions ...t/apps/core/fd/components/form/fileinput/v2/fileinput/clientlibs/site/js/fileinputview.js
View file
Open in desktop
Expand Up @@ -86,6 +86,20 @@
return this.element.querySelector(FileInputV2.selectors.attachButtonLabel);
}

updateEnabled(enabled, state) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

updateEnabled(enabled, state) {

    // Update data attribute for enabled state
    this.element.setAttribute(FormView.Constants.DATA_ATTRIBUTE_ENABLED, enabled);
    
    // Determine if the widget should be disabled
    const isDisabled = !enabled || state.readOnly;

    // Update the 'disabled' and ARIA 'disabled' attributes based on the enabled state
    if (isDisabled) {
        this.widget.setAttribute("disabled", "disabled");
        this.widget.setAttribute(FormView.Constants.ARIA_DISABLED, "true");
    } else {
        this.widget.removeAttribute("disabled");
        this.widget.removeAttribute(FormView.Constants.ARIA_DISABLED);
    }
}

Always try to create a constant inside the if condition to make the code more readable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, implemented the same

if (this.widget) {
this.element.setAttribute(FormView.Constants.DATA_ATTRIBUTE_ENABLED, enabled);
const isDisabled = !enabled || state.readOnly;
if (isDisabled) {
this.widget.setAttribute("disabled", "disabled");
this.widget.setAttribute(FormView.Constants.ARIA_DISABLED, true);
} else {
this.widget.removeAttribute("disabled");
this.widget.removeAttribute(FormView.Constants.ARIA_DISABLED);
}
}
}

updateValue(value) {
if (this.widgetObject == null) {
this.widgetObject = new FileInputWidget(this.widgetFields);
Expand Down
3 changes: 1 addition & 2 deletions .../core/fd/components/form/radiobutton/v1/radiobutton/clientlibs/site/js/radiobuttonview.js
View file
Open in desktop
Expand Up @@ -94,7 +94,6 @@
}

updateEnabled(enabled, state) {
this.toggle(enabled, FormView.Constants.ARIA_DISABLED, true);
this.element.setAttribute(FormView.Constants.DATA_ATTRIBUTE_ENABLED, enabled);
let widgets = this.widget;
widgets.forEach(widget => {
Expand All @@ -110,7 +109,7 @@
});
}

updateReadOnly(readonly, state) {
updateReadOnly(readonly) {
let widgets = this.widget;
this.element.setAttribute(FormView.Constants.DATA_ATTRIBUTE_READONLY, readonly);
widgets.forEach(widget => {
Expand Down
3 changes: 1 addition & 2 deletions ui.frontend/src/view/FormFieldBase.js
View file
Open in desktop
Expand Up @@ -415,7 +415,6 @@ class FormFieldBase extends FormField {
*/
updateEnabled(enabled, state) {
if (this.widget) {
this.toggle(enabled, Constants.ARIA_DISABLED, true);
this.element.setAttribute(Constants.DATA_ATTRIBUTE_ENABLED, enabled);
if (enabled === false) {
this.widget.setAttribute("disabled", "disabled");
Expand All @@ -432,7 +431,7 @@ class FormFieldBase extends FormField {
* @param {boolean} readOnly - The read-only state.
* @param {Object} state - The state object.
*/
updateReadOnly(readOnly, state) {
updateReadOnly(readOnly) {
if (this.widget) {
this.element.setAttribute(Constants.DATA_ATTRIBUTE_READONLY, readOnly);
if (readOnly === true) {
Expand Down
16 changes: 13 additions & 3 deletions ui.tests/test-module/specs/fileinput/fileinput.runtime.spec.js
View file
Open in desktop
Expand Up @@ -80,7 +80,7 @@ describe("Form with File Input - Basic Tests", () => {
const checkHTML = (id, state) => {
const visible = state.visible;
const passVisibleCheck = `${visible === true ? "" : "not."}be.visible`;
const passDisabledAttributeCheck = `${state.enabled === false ? "" : "not."}have.attr`;
const passDisabledAttributeCheck = `${state.enabled === false || state.readOnly === true ? "" : "not."}have.attr`;
Copy link
Collaborator

Choose a reason for hiding this comment

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

As discussed in the scrum, can you please add the same check in all the test specs for all the components ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure Rishi, Will be adding the test cases in the next pr which I am working on for tnc component

const value = (state.value == null ? '' : (Array.isArray(state.value) ? state.value[0].name : state.value.name)); // check for file name in dom
cy.get(`#${id}`)
.should(passVisibleCheck)
Expand Down Expand Up @@ -138,7 +138,12 @@ describe("Form with File Input - Basic Tests", () => {
if (model.visible && model.enabled) {
cy.get(`#${id}`).should('have.class', 'cmp-adaptiveform-fileinput--empty');
cy.get(`#${id}`).invoke('attr', 'data-cmp-required').should('eq', 'false');
cy.get(`#${id}`).invoke('attr', 'data-cmp-readonly').should('eq', 'false');
if(model.readOnly) {
cy.get(`#${id}`).should('have.attr', 'data-cmp-readonly', 'true');
} else {
cy.get(`#${id}`).should('have.attr', 'data-cmp-readonly', 'false');
}
//cy.get(`#${id}`).invoke('attr', 'data-cmp-readonly').should('eq', 'false');
cy.get(`#${id}`).find("input").attachFile(fileName).then(x => {
let expectedFileName = Array.isArray(model.getState().value) ? model.getState().value[0].name : model.getState().value.name;
expect(expectedFileName).to.equal(fileName)
Expand All @@ -148,7 +153,7 @@ describe("Form with File Input - Basic Tests", () => {
}
}
});
getFormObjTest(['empty.pdf', 'empty.pdf', 'empty.pdf', 'empty.pdf'])
getFormObjTest(['empty.pdf', 'empty.pdf', 'empty.pdf', 'empty.pdf', 'empty.pdf'])
});

it("should toggle description and tooltip", () => {
Expand Down Expand Up @@ -177,6 +182,11 @@ describe("Form with File Input - Basic Tests", () => {
cy.get(".cmp-adaptiveform-fileinput__widgetlabel").should('have.attr', 'role', 'button');
})

it(`fielinput is disabled when readonly property is true`, () => {
const fileInput5 = "input[name='fileinput5']";
cy.get(fileInput5).should("have.attr", "disabled", "disabled");
});

})

describe("Form with File Input - Prefill & Submit tests", () => {
Expand Down