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] website: remove navbar extra padding #163705

Closed
wants to merge 1 commit into from

Conversation

imanie383
Copy link
Contributor

@imanie383 imanie383 commented Apr 28, 2024

Description of the issue/feature this PR addresses:
In the mobile view, the header has extra padding, for this reason it is not perfectly aligned with the content of the page.

Current behavior before PR:

The header is not aligned with the content

Desktop Mobile
image image

Desired behavior after PR is merged:

The header is aligned with the content like desktop view

2024-04-28.09-07-05.mp4

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

@robodoo
Copy link
Contributor

robodoo commented Apr 28, 2024

In the mobile view, the header has extra padding, for this
reason it is not perfectly aligned with the content of the page.
@C3POdoo C3POdoo requested a review from a team April 28, 2024 15:14
Copy link
Contributor

@rdeodoo rdeodoo left a comment

Choose a reason for hiding this comment

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

@qsm-odoo This one should make you happy :o
Visually LGTM

Copy link
Contributor

@qsm-odoo qsm-odoo left a comment

Choose a reason for hiding this comment

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

Let's go with that in 17.0. In master, we'll just change the use of $navbar-padding-x. And later maybe do even better (task-2241779).

Thanks ! 👍

@robodoo r+

@imanie383
Copy link
Contributor Author

imanie383 commented Apr 30, 2024

I didn't want to use $navbar-padding-x because we would modify the padding of all the navs(event, slides, etc )

@imanie383
Copy link
Contributor Author

Let's go with that in 17.0. In master, we'll just change the use of $navbar-padding-x. And later maybe do even better (task-2241779).

I want to create the fix for 16, @qsm-odoo

do you want me to use $navbar-padding-x variable?

@qsm-odoo
Copy link
Contributor

I didn't want to use $navbar-padding-x because we would modify the padding of all the navs(event, slides, etc )

Yes, that's good, that's why I merged it 👍

I want to create the fix for 16, @qsm-odoo
do you want me to use $navbar-padding-x variable?

No, I am changing that for the master version but we should not in stable versions. I actually found other bugs when navbar-padding-x is equal to 0, I am fixing for master. For master, forcing the main navbar padding with a padding class is good 👍

willylohws pushed a commit to willylohws/odoo that referenced this pull request May 1, 2024
In the mobile view, the header has extra padding, for this
reason it is not perfectly aligned with the content of the page.

closes odoo#163705

Signed-off-by: Quentin Smetz (qsm) <qsm@odoo.com>
@fw-bot
Copy link
Contributor

fw-bot commented May 4, 2024

@imanie383 @qsm-odoo this pull request has forward-port PRs awaiting action (not merged or closed):

2 similar comments
@fw-bot
Copy link
Contributor

fw-bot commented May 5, 2024

@imanie383 @qsm-odoo this pull request has forward-port PRs awaiting action (not merged or closed):

@fw-bot
Copy link
Contributor

fw-bot commented May 6, 2024

@imanie383 @qsm-odoo this pull request has forward-port PRs awaiting action (not merged or closed):

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants