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

[Cleanup] Adjusting Company Switcher Per New Design #2330

Open
wants to merge 13 commits into
base: develop
Choose a base branch
from
70 changes: 70 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
"pusher-js": "^8.4.0-rc2",
"randexp": "^0.5.3",
"react": "^18.2.0",
"react-avatar": "^5.0.3",
"react-colorful": "^5.6.1",
"react-date-range": "^1.4.0",
"react-datepicker": "^4.11.0",
Expand Down
37 changes: 37 additions & 0 deletions src/common/hooks/useHandleCollapseExpandSidebar.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/**
* Invoice Ninja (https://invoiceninja.com).
*
* @link https://github.com/invoiceninja/invoiceninja source repository
*
* @copyright Copyright (c) 2022. Invoice Ninja LLC (https://invoiceninja.com)
*
* @license https://www.elastic.co/licensing/elastic-license
*/

import { useUpdateCompanyUser } from '$app/pages/settings/user/common/hooks/useUpdateCompanyUser';
import { cloneDeep, set } from 'lodash';
import { useHandleCurrentUserChangeProperty } from './useHandleCurrentUserChange';
import { useInjectUserChanges } from './useInjectUserChanges';

export function useHandleCollapseExpandSidebar() {
const userChanges = useInjectUserChanges();

const updateCompanyUser = useUpdateCompanyUser();
const handleUserChange = useHandleCurrentUserChangeProperty();

return (value: boolean) => {
handleUserChange('company_user.react_settings.show_mini_sidebar', value);

if (userChanges) {
const updatedUserChanges = cloneDeep(userChanges);

set(
updatedUserChanges,
'company_user.react_settings.show_mini_sidebar',
value
);

updateCompanyUser(updatedUserChanges);
}
};
}
2 changes: 1 addition & 1 deletion src/common/hooks/useInjectUserChanges.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export function useInjectUserChanges(options?: Options) {
const changes = useUserChanges();

useEffect(() => {
if (changes && options?.overwrite === false) {
Copy link
Collaborator Author

@Civolilah Civolilah Jan 30, 2025

Choose a reason for hiding this comment

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

@beganovich I don't think this ever worked correctly. David reported that the speed of collapsing/expanding sidebar is really slow - the reason for that is exactly this logic. The changes object is constantly being injected again because this if statement is almost never true.

By default, the changes object used in this if statement is an empty object ({}), it is never null/undefined. So, that part of the condition is always true, while the second part is equal to false only if the overwrite is passed through options. So, instead of comparing types also, we just want to check if the overwrite is set to true.

The overwrite is never passed as a true value, but also I don't think we ever need to overwrite changes over the changes we currently have.

The same thing applies to useInjectCompanyChange - the overwrite never passes the check that is false.

if (Object.keys(changes || {}).length && !options?.overwrite) {
// We don't want to overwrite existing changes,
// so let's just not inject anything if we already have a value,
// and relative argument.
Expand Down
Loading
Loading