Skip to content

Commit

Permalink
address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
EshaanAgg committed Mar 5, 2024
1 parent ecd4cc5 commit e6dfa0c
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 33 deletions.
2 changes: 1 addition & 1 deletion benchexec/tablegenerator/react-table/build/main.min.js

Large diffs are not rendered by default.

17 changes: 4 additions & 13 deletions benchexec/tablegenerator/react-table/src/components/ReactTable.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import {
emptyStateValue,
isNil,
hasSameEntries,
setHashSearch,
setParam,
getHashSearch,
getHiddenColIds,
decodeFilter,
Expand Down Expand Up @@ -589,29 +589,20 @@ const Table = (props) => {
)
.join(";");
const value = sort.length ? sort : undefined;
const prevParams = getHashSearch();
if (prevParams["sort"] !== value) {
setHashSearch({ sort: value }, true);
}
setParam({ sort: value });
}, [sortBy]);

// Update the URL page size param when the table page size setting changed
useEffect(() => {
const value = pageSize !== initialPageSize ? pageSize : undefined;
const prevParams = getHashSearch();
if (prevParams["pageSize"] !== value) {
setHashSearch({ pageSize: value }, true);
}
setParam({ pageSize: value });
}, [pageSize]);

// Update the URL page param when the table page changed
useEffect(() => {
const value =
pageIndex && pageIndex !== 0 ? Number(pageIndex) + 1 : undefined;
const prevParams = getHashSearch();
if (prevParams["page"] !== value) {
setHashSearch({ page: value }, true);
}
setParam({ page: value }, true);
}, [pageIndex]);

// Store the column resizing values so they can be applied again in case the table rerenders
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,10 @@ export default class SelectColumn extends React.Component {
hiddenParams["hidden"] = null;
}

setHashSearch(hiddenParams, true, this.props.history);
setHashSearch(hiddenParams, {
keepOthers: true,
history: this.props.history,
});
}

// -------------------------Rendering-------------------------
Expand Down
22 changes: 16 additions & 6 deletions benchexec/tablegenerator/react-table/src/tests/utils.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
constructQueryString,
decodeFilter,
hasSameEntries,
getHashURL,
constructHashURL,
makeFilterSerializer,
makeFilterDeserializer,
splitUrlPathForMatchingPrefix,
Expand Down Expand Up @@ -128,13 +128,13 @@ describe("hashRouting helpers", () => {
});
});

describe("getHashURL", () => {
describe("constructHashURL", () => {
test("should construct URL hash with provided parameters", () => {
const baseUrl = "http://example.com";
const params = { key1: "value1", key2: "value2" };
const keepOthers = false;

expect(getHashURL(baseUrl, params, keepOthers)).toEqual(
expect(constructHashURL(baseUrl, params, keepOthers)).toEqual(
"http://example.com?key1=value1&key2=value2",
);
});
Expand All @@ -144,7 +144,7 @@ describe("hashRouting helpers", () => {
const params = { key1: "value1", key2: "value2" };
const keepOthers = true;

expect(getHashURL(baseUrl, params, keepOthers)).toEqual(
expect(constructHashURL(baseUrl, params, keepOthers)).toEqual(
"http://example.com?existingKey=existingValue&key1=value1&key2=value2",
);
});
Expand All @@ -154,7 +154,7 @@ describe("hashRouting helpers", () => {
const params = {};
const keepOthers = true;

expect(getHashURL(baseUrl, params, keepOthers)).toEqual(
expect(constructHashURL(baseUrl, params, keepOthers)).toEqual(
"http://example.com?exisitingKey=existingValue",
);
});
Expand All @@ -164,10 +164,20 @@ describe("hashRouting helpers", () => {
const params = {};
const keepOthers = false;

expect(getHashURL(baseUrl, params, keepOthers)).toEqual(
expect(constructHashURL(baseUrl, params, keepOthers)).toEqual(
"http://example.com",
);
});

test("should override existing parameters with new ones", () => {
const baseUrl = "http://example.com?key1=value1&key2=value2";
const params = { key2: "newValue" };
const keepOthers = true;

expect(constructHashURL(baseUrl, params, keepOthers)).toEqual(
"http://example.com?key1=value1&key2=newValue",
);
});
});
});

Expand Down
29 changes: 17 additions & 12 deletions benchexec/tablegenerator/react-table/src/utils/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,9 @@ const getHashSearch = (str) => {
const search = urlParts.length > 1 ? urlParts.slice(1).join("?") : undefined;

// If there are no search parameters, return an empty object
if (search === undefined || search.length === 0) return {};
if (search === undefined || search.length === 0) {
return {};
}

// Split the search string into key-value pairs and generate an object from them
const keyValuePairs = search.split("&").map((pair) => pair.split("="));
Expand Down Expand Up @@ -249,7 +251,7 @@ export const constructQueryString = (params) => {
* @param {boolean} [keepOthers=false] - Whether to keep existing parameters in the URL hash or not
* @returns {string} - The constructed URL hash
*/
export const getHashURL = (url, params = {}, keepOthers = false) => {
export const constructHashURL = (url, params = {}, keepOthers = false) => {
const additionalParams = keepOthers ? getHashSearch(url) : {};
const mergedParams = { ...additionalParams, ...params };

Expand All @@ -263,17 +265,18 @@ export const getHashURL = (url, params = {}, keepOthers = false) => {
* Sets or updates the search parameters in the URL hash of the current page.
*
* @param {Object} params - The parameters to be set or updated in the URL hash
* @param {boolean} [keepOthers=false] - Whether to keep existing parameters in the URL hash or not
* @param {Object} [history=undefined] - The history object to be used for navigation
* @param {boolean} [options={}] - The options to be used for setting the URL hash
* @param {Object} [options.history=null] - The history object to be used for updating the URL hash
* @param {boolean} [options.keepOthers=false] - Whether to keep existing parameters in the URL hash or not
* @returns {void}
*/
const setHashSearch = (
params = {},
keepOthers = false,
history = undefined,
) => {
const newUrl = getHashURL(document.location.href, params, keepOthers);
if (history) history.push(newUrl);
const setHashSearch = (params = {}, options = {}) => {
const { history = null, keepOthers = false } = options;
const newUrl = constructHashURL(document.location.href, params, keepOthers);

if (history && history.push) {
history.push(newUrl);
}
document.location.href = newUrl;
};

Expand Down Expand Up @@ -682,7 +685,9 @@ const setConstantHashSearch = (paramString) => {
* @param {Object} param The Key-Value pair to be added to the current query param list
*/
const setParam = (param) => {
setHashSearch(param, true);
setHashSearch(param, {
keepOthers: true,
});
};

const stringAsBoolean = (str) => str === "true";
Expand Down

0 comments on commit e6dfa0c

Please sign in to comment.