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

feat(component): add enabled prop to within the columns entity (worksheet) #1241

Merged
merged 1 commit into from
Jul 11, 2023

Conversation

bc-apostoliuk
Copy link
Contributor

What?

Add enabled prop to within the columns entity (worksheet).

Fix the prop naming in the docs (disable -> disabled)

Why?

To allow the enablement of the columns for the case when the rows are disabled.

Screenshots/Screen Recordings

Screenshot 2023-06-26 at 18 14 43

Testing/Proof

Locally.

@bc-apostoliuk bc-apostoliuk requested review from a team as code owners June 26, 2023 15:17
@chanceaclark
Copy link
Contributor

What is the use-case for adding enabled? Why can't we use !disabled?

@bc-apostoliuk bc-apostoliuk force-pushed the col-enable branch 2 times, most recently from a5b8159 to 0b18a93 Compare June 26, 2023 16:34
@bc-apostoliuk
Copy link
Contributor Author

bc-apostoliuk commented Jun 26, 2023

@chanceaclark
Currently, if the row is disabled -> all cells within this row are disabled. And if we pass disabled: false for some column it will not enable this column, since the full row is disabled.
Actually, we can rework the current logic of the disabled field of the column and handle the disabled: false as the highest priority over the disabled rows. I have created the additional property to avoid confusion.

What do you think, do we need to set the highest priority for the disabled prop within the column, and handle disabled: false like this column always should be available independently of disabled rows?

@bc-apostoliuk bc-apostoliuk force-pushed the col-enable branch 2 times, most recently from fb84f2c to 2317885 Compare June 26, 2023 19:19
@bc-apostoliuk bc-apostoliuk merged commit aaad5cb into bigcommerce:master Jul 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants