[Cleanup] Adjusting Company Switcher Per New Design #2330
Conversation
|
@turbo124 The only color I used from Figma is for the icons of add_company, logout, and other actions. It looks really great to me in both light and dark mode - better than any other from the color scheme. All other colors are used from the existing scheme. |
|
After minimising the sidebar, how do we re-expand it? cc @beganovich ok I found it:
Problem. it is not very intuitive for me (perhaps we need to relocate this to bottom so that it is always in context. Also, i note a huge delay when clicking to reopen the sidebar
I think this is related to the API call for updating the react_settings for this, we should not need to await this. |
@turbo124 The icon has been replaced and the expanding/collapsing sidebar speed has been improved. Let me now your thoughts. |
| @@ -31,7 +31,7 @@ export function useInjectUserChanges(options?: Options) { | |||
| const changes = useUserChanges(); | |||
|
|
|||
| useEffect(() => { | |||
| if (changes && options?.overwrite === false) { | |||
There was a problem hiding this comment.
@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.
|
both change buttons need to be in the same location to keep the UX consistent, place them both in the footer section. |
@turbo124 Ah, sure thing! It has been replaced! |
|
If no company logo present we should a character, can we please ensure this sizes proportionately to fill the circle. |
@beganovich Ah sure, I didn't notice this one because I didn't have that many icons. The sizes of icons have been decreased and spacing has been increased. Screenshots:
Let me know your thoughts. |
|
Quick change needed on this one, if you don't have profile picture, and use invoice ninja default one, this is how it looks.
|
@beganovich Ah sure thing, it has been fixed. Screenshot:
Let me know your thoughts. |
package.json
Outdated
| @@ -38,6 +38,7 @@ | |||
| "pusher-js": "^8.4.0-rc2", | |||
| "randexp": "^0.5.3", | |||
| "react": "^18.2.0", | |||
| "react-avatar": "^5.0.3", | |||
There was a problem hiding this comment.
We definitely need to kick out this library. It wasn't updated in years and it will definitely block our path to React 19.
As replacement we can use ui-avatars.com, but we should be careful here as well.
@turbo124 should we use 3rd party website to generate nice avatars for us (due to privacy concerns).
Just to be clear react-avatar isn't fully offline either.
src/components/CompanySwitcher.tsx
Outdated
| className={classNames( | ||
| 'rounded-full border overflow-hidden aspect-square', | ||
| { | ||
| 'object-cover': logo?.includes('invoiceninja-logo@light'), |
There was a problem hiding this comment.
I am not sure this is correct way to handle this, we need same approach for every avatar. Both rounded, 1:1 aspect or 16:9.
Can we apply same this for everyone and test different sizes?
There was a problem hiding this comment.
@beganovich Okay, I made the same approach for all avatars to match the Figma design with the circle and logo. The best preview will be for 1:1 pictures, of course. Screenshot:
However, if we want to have a consistent design, 16:9 pictures for example will be less visible, but I don't think there is a general approach to make all of them look great when they are displayed within the same avatar shape. This is how it looks with a 16:9 picture. Screenshot:
It is not fully visible of course, but I'm not sure if it's even possible when the avatar is consistent and circular. Also, in my opinion, the avatar is a small space to make it visible for all sizes, and I don't think that an avatar is actually a place where it must be fully visible?
Let me know your thoughts.
There was a problem hiding this comment.
Not to worry, optimize for 1:1 aspect ration, for 16:9 we can cut the content.
|
Let's dismiss the idea of dynamic avatars with 3rd party sites or packages. Can we default to a circular Invoice Ninja logo for users who don't have avatars? |
@beganovich Hmm, okay, I think it looks better when the letter avatar is displayed instead of the default Invoice Ninja logo. Screenshot:
Do you think this would be a good solution? Here's how it would look with this change. |
|
Please use this logo: https://github.com/invoiceninja/ui/blob/main/public/logo180.png |
@beganovich Sure thing, I made it as a fallback logo and it looks much better now. Screenshot:
Let me know your thoughts. |
@beganovich @turbo124 The PR includes the redesign of the company switcher using the existing color scheme and font sizes but with a new design. Screenshot:
Let me know your thoughts.