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

[16.0] [MIG] web_sheet_full_width #2327

Merged
merged 33 commits into from
Feb 21, 2023

Conversation

hugosantosred
Copy link
Member

Migration of web_sheet_full_width to v16

#2309

Works as expected without other changes.

@hugosantosred hugosantosred force-pushed the 16.0-mig-web_sheet_full_width branch 2 times, most recently from 1d10f64 to 323f7ef Compare October 28, 2022 09:23
@pedrobaeza
Copy link
Member

Not sure if we should pack all the moddings to the default responsive theme in one module:

  • This full width.
  • App switcher.
  • etc

@SplashS what do you think? Are you working on something?

@LoisRForgeFlow
Copy link
Contributor

@pedrobaeza This module is still useful to use in conbination with EE.

Copy link
Contributor

@LoisRForgeFlow LoisRForgeFlow left a comment

Choose a reason for hiding this comment

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

Functional test 👍

Copy link
Member

@tarteo tarteo left a comment

Choose a reason for hiding this comment

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

Installed locally, but I don't see any changes to the width of the sheet.

image

@flotho
Copy link
Member

flotho commented Dec 22, 2022

any chance to retrigger the runboat build

@baimont
Copy link
Contributor

baimont commented Jan 5, 2023

Installed locally, but I don't see any changes to the width of the sheet.

image

Hi there

Installed locally on an enterprise version.
It works for me.

For a large width I get the same result as @tarteo, which is the standard way.
image

But when you reduce the width so that the chatter is displayed below the sheet, the sheet takes all the width of the window.
image

This is different from the standard.
image

So functional test ok for me! Thanks for this pr.

:target: http://www.gnu.org/licenses/agpl-3.0-standalone.html
:alt: License: AGPL-3
.. |badge3| image:: https://img.shields.io/badge/github-OCA%2Fweb-lightgray.png?logo=github
:target: https://github.com/OCA/web/tree/15.0/web_sheet_full_width
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know when and how the 15.0 mentions will be replaced by 16.0, maybe when this is merged?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't know either. I'm going to check it in the case I missed to change it manually

Copy link
Member Author

Choose a reason for hiding this comment

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

The README.rst its generated again when is merged by oca-bot so that mentions to 15.0 will be replaced then.

Copy link
Contributor

@hieulucky111 hieulucky111 left a comment

Choose a reason for hiding this comment

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

LGTM

@flotho
Copy link
Member

flotho commented Jan 6, 2023

runboat stucked, any chance to retrigger an event for testing on runboat

@hugosantosred
Copy link
Member Author

@tarteo That behaviour with the chatter on the right is the default for large width. When web_chatter_position gets migrated this behaviour could be configured to show the chatter always on the bottom.

I've just tried by removing the chatter manually in a large width:

  • With the standard behaviour

image

  • With web_sheet_full_width installed

image

@hugosantosred hugosantosred force-pushed the 16.0-mig-web_sheet_full_width branch from 323f7ef to f336757 Compare January 16, 2023 09:30
@liebana
Copy link

liebana commented Feb 3, 2023

Functionally tested, LGTM

Copy link

@antoniocanovas antoniocanovas left a comment

Choose a reason for hiding this comment

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

Functional review Ok

@hugosantosred hugosantosred requested a review from tarteo February 17, 2023 10:12
@tarteo
Copy link
Member

tarteo commented Feb 21, 2023

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 16.0-ocabot-merge-pr-2327-by-tarteo-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 470f1f4 into OCA:16.0 Feb 21, 2023
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 1db111e. Thanks a lot for contributing to OCA. ❤️

@hugosantosred hugosantosred deleted the 16.0-mig-web_sheet_full_width branch February 23, 2023 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.