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

fix: remove collapsible label prefix from the FieldSelect dropdown component #10760

Open
wants to merge 3 commits into
base: 2.x
Choose a base branch
from

Conversation

wrobson-lllow
Copy link

@wrobson-lllow wrobson-lllow commented Jan 23, 2025

What?

Removes Collapsible label from child field label in the FieldSelect component (for example Bulk Edit field select dropdown)

Why?

Including the Collapsible field label in the option label of the FieldSelect component could potentially be confusing to a user as it eludes to the field being nested in the Data schema.

How?

In the reduceFields function of the FieldsLabel component, if field.type === 'collpasible' the collapsible field's label will not be combined with the child fields labels.

Fixes #10757

@PatrikKozak PatrikKozak changed the title fix(collabsible-label-fix): remove collapsible label prefix fix: remove collapsible label prefix from the FieldSelect dropdown component Jan 27, 2025
@PatrikKozak PatrikKozak self-requested a review January 27, 2025 19:39
@wrobson-lllow
Copy link
Author

@PatrikKozak Does anything else need to be done by me with this? Thanks.

@PatrikKozak
Copy link
Contributor

Hey @wrobson-lllow, I've gone ahead and reviewed / tested this locally and it looks great!

I haven't had the chance to look into why those tests are failing in the CI though but I don't think they're related to your fix.

Could you try re-pulling the 2.x branch into your PR branch?

If that doesn't fix the CI tests from failing without actually running, you might need to re-open a new PR with the above changes.

One final note: could you add an e2e test for this? Should look something along the lines of this:

test('should not render collapsible label in bulk update field select', async () => {
   await page.goto(url.list)

   await page.locator('input#select-all').check()
   await page.locator('.edit-many__toggle').click()
   await page.locator('.field-select .rs__control').click()

   const firstCollapsibleOption = page.locator('.rs__option', {
     hasText: exactText('Text'),
   })

   await expect(firstCollapsibleOption).toBeVisible()
})

If neither of the two suggestions above fix the CI, I'll look into this further! Thanks again!

@wrobson-lllow
Copy link
Author

@PatrikKozak Will do, I should have some time to work on that this afternoon. Thanks!

1 similar comment
@wrobson-lllow
Copy link
Author

@PatrikKozak Will do, I should have some time to work on that this afternoon. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants