Skip to content

Commit

Permalink
Only ignore hiding required columns in dynamic mode (streamlit#7996)
Browse files Browse the repository at this point in the history
* Only ignore hiding required columns in dynamic mode

* Add test

* Update test
  • Loading branch information
LukasMasuch authored and zyxue committed Apr 16, 2024
1 parent 4141130 commit f1033da
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ describe("useColumnLoader hook", () => {
}
})

it("disallows hidden for editable columns that are required", () => {
it("disallows hidden for editable columns that are required for dynamic editing", () => {
const element = ArrowProto.create({
data: UNICODE,
editingMode: ArrowProto.EditingMode.DYNAMIC,
Expand All @@ -381,6 +381,29 @@ describe("useColumnLoader hook", () => {
expect(result.current.columns[1].isHidden).toBe(false)
})

it("respects hiding required columns for fixed editing", () => {
const element = ArrowProto.create({
data: UNICODE,
editingMode: ArrowProto.EditingMode.FIXED,
columns: JSON.stringify({
c1: {
required: true,
hidden: true,
},
}),
})

const data = new Quiver(element)

const { result } = renderHook(() => {
return useColumnLoader(element, data, false)
})

// Test that the column is hidden (not part of columns).
// Column with index 1 should be c2:
expect(result.current.columns[1].name).toBe("c2")
})

it("doesn't configure any icon for non-editable columns", () => {
const element = ArrowProto.create({
data: UNICODE,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -263,8 +263,11 @@ function useColumnLoader(
icon: "editable",
}

// Make sure that required columns are not hidden:
if (updatedColumn.isRequired) {
// Make sure that required columns are not hidden when editing mode is dynamic:
if (
updatedColumn.isRequired &&
element.editingMode === ArrowProto.EditingMode.DYNAMIC
) {
updatedColumn = {
...updatedColumn,
isHidden: false,
Expand Down

0 comments on commit f1033da

Please sign in to comment.