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

[IMP] web: Always show the name of the company on the app top right #160057

Closed

Conversation

hsal-odoo
Copy link
Contributor

@hsal-odoo hsal-odoo commented Apr 1, 2024

Removed the isDisplayed condition of the SwitchCompanyMenu component.
With this, the name of the company on the top right of the app will always be shown even is there is only one company.
Also, if there is a single company, the company selector will be disabled.

The name of the company on the top right of the app was hidden when there was only one company.

task-3829645


I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

@hsal-odoo hsal-odoo requested a review from malb-odoo April 1, 2024 06:56
@robodoo
Copy link
Contributor

robodoo commented Apr 1, 2024

Pull request status dashboard.

@hsal-odoo hsal-odoo force-pushed the master-web-show_company_name-hsal branch from 9925fba to baacdbb Compare April 1, 2024 06:57
@hsal-odoo hsal-odoo marked this pull request as ready for review April 1, 2024 06:57
@C3POdoo C3POdoo requested review from a team, aab-odoo and Iucapad and removed request for a team April 1, 2024 06:59
@C3POdoo C3POdoo added the RD research & development, internal work label Apr 1, 2024
@hsal-odoo hsal-odoo force-pushed the master-web-show_company_name-hsal branch from baacdbb to 31575da Compare April 11, 2024 02:38
Copy link
Contributor

@malb-odoo malb-odoo left a comment

Choose a reason for hiding this comment

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

LGTM Thanks for this 😄 Just maybe a question about the company selector for just one company
image

Since Olma Approved it i suppose it's good but still maybe Quentin can told us

@qdp-odoo
Copy link
Contributor

yeah, I think @Olivier-olma simply missed it. Well seen @malb-odoo . I propose to replace the company selector with just a label if len(companies) == 1

@Olivier-olma
Copy link

You are right, I missed it, thanks @malb-odoo

@hsal-odoo hsal-odoo force-pushed the master-web-show_company_name-hsal branch from 31575da to 8e35859 Compare April 24, 2024 10:46
@hsal-odoo
Copy link
Contributor Author

@qdp-odoo should be fixed now

@hsal-odoo hsal-odoo force-pushed the master-web-show_company_name-hsal branch 2 times, most recently from c542201 to f84e57f Compare May 29, 2024 14:37
@hsal-odoo hsal-odoo requested a review from malb-odoo May 29, 2024 14:37
Copy link
Contributor

@malb-odoo malb-odoo left a comment

Choose a reason for hiding this comment

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

LGTM thanks for this ! 😄

Copy link
Contributor

@aab-odoo aab-odoo left a comment

Choose a reason for hiding this comment

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

Hello. This should be tested

@hsal-odoo hsal-odoo force-pushed the master-web-show_company_name-hsal branch 2 times, most recently from 4695d88 to cce9d50 Compare May 30, 2024 11:36
@hsal-odoo
Copy link
Contributor Author

@aab-odoo @malb-odoo I have added a unit test

@hsal-odoo hsal-odoo force-pushed the master-web-show_company_name-hsal branch 2 times, most recently from 72437df to 9ca7d9c Compare May 31, 2024 09:13
@hsal-odoo hsal-odoo requested a review from aab-odoo May 31, 2024 09:13
@BastienFafchamps
Copy link
Contributor

LGTM as well!
@robodoo r+

@robodoo
Copy link
Contributor

robodoo commented May 31, 2024

@BastienFafchamps you may want to rebuild or fix this PR as it has failed CI.

@hsal-odoo hsal-odoo force-pushed the master-web-show_company_name-hsal branch 3 times, most recently from 9f9399c to 99c8c8f Compare June 3, 2024 11:36
Removed the isDisplayed condition of the SwitchCompanyMenu component.
With this, the name of the company on the top right of the app will always be shown even is there is only one company.
Also, if there is a single company, the company selector will be disabled.

The name of the company on the top right of the app was hidden when there was only one company.

task-3829645
@hsal-odoo hsal-odoo force-pushed the master-web-show_company_name-hsal branch from 99c8c8f to 8adaa1b Compare June 3, 2024 17:15
@hsal-odoo
Copy link
Contributor Author

hsal-odoo commented Jun 3, 2024

@BastienFafchamps Please review my last update. I have only added nullish coalescing operator for allowedCompaniesWithAncestors This fix was necessary for the runbot to pass enterprise tests

@qdp-odoo
Copy link
Contributor

qdp-odoo commented Jun 7, 2024

@robodoo r+

@hsal-odoo thanks!
the commit message could have been clearer, tho: the last sentence "The name of the company on the top right of the app was hidden when there was only one company." is kinda redundant, misplaced, and useless... IMAO 😅

robodoo pushed a commit that referenced this pull request Jun 7, 2024
Removed the isDisplayed condition of the SwitchCompanyMenu component.
With this, the name of the company on the top right of the app will always be shown even is there is only one company.
Also, if there is a single company, the company selector will be disabled.

The name of the company on the top right of the app was hidden when there was only one company.

task-3829645

closes #160057

Signed-off-by: Quentin De Paoli <qdp@odoo.com>
@robodoo robodoo closed this Jun 7, 2024
@robodoo robodoo added the 17.4 label Jun 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
17.4 RD research & development, internal work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants