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.

Sign up for GitHub

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

Jump to bottom

[12.0][IMP] web_responsive: Document Viewer #1467

Merged
merged 1 commit into from Jan 3, 2020
Merged

[12.0][IMP] web_responsive: Document Viewer #1467

merged 1 commit into from Jan 3, 2020

Conversation

Tardo
Copy link
Member

@Tardo Tardo commented Dec 24, 2019 •
edited

document_viewer

cc @Tecnativa TT21158

@Tardo Tardo changed the title [IMP] web_responsive: Document Viewer [12.0][IMP] web_responsive: Document Viewer Dec 24, 2019
@pedrobaeza pedrobaeza requested a review from yajo December 31, 2019 13:16
@pedrobaeza pedrobaeza added this to the 12.0 milestone Jan 2, 2020
pedrobaeza
pedrobaeza reviewed Jan 2, 2020
View reviewed changes
Copy link
Member

@pedrobaeza pedrobaeza left a comment •
edited

Choose a reason for hiding this comment

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

Excellent new feature!

We need to document it on the README file as the other ones, and the GIF is perfect for being included. You can put this text:

* When the chatter is configured on the side part, the document viewer fills that 
  part for side-by-side reading instead of full screen. You can still put it on full 
  width preview clicking on the new maximize button.

web_responsive/static/src/css/web_responsive.scss Outdated Show resolved Hide resolved
@pedrobaeza pedrobaeza mentioned this pull request Jan 2, 2020
Merged
yajo
yajo requested changes Jan 2, 2020
View reviewed changes
Copy link
Member

@yajo yajo left a comment

Choose a reason for hiding this comment

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

This needs comments.

Nice feature OTOH!

web_responsive/static/src/css/web_responsive.scss Outdated Show resolved Hide resolved
web_responsive/static/src/js/web_responsive.js Outdated Show resolved Hide resolved
web_responsive/static/src/xml/document_viewer.xml Outdated Show resolved Hide resolved
web_responsive/views/web.xml Show resolved Hide resolved
web_responsive/static/src/css/web_responsive.scss Outdated Show resolved Hide resolved
@Tardo Tardo force-pushed the 12.0-imp-web_responsive-preview_documents branch from 87b0c9a to 156dd18 Compare January 2, 2020 15:18
@Tardo
Copy link
Member Author

Tardo commented Jan 2, 2020

Changes done! Thanks for the reviews... and thanks @pedrobaeza for the original idea!

pedrobaeza
pedrobaeza approved these changes Jan 2, 2020
View reviewed changes
Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

OK by my side.

@legalsylvain
Copy link
Contributor

Hi @Tardo. Thanks a lot for this feature. looks very good !
I'd like to test, but runbot link is not available and there are no link here : https://runbot.odoo-community.org/
@gurneyalex : An idea ? (after your vacation, of course ;-))

@pedrobaeza
Copy link
Member

After the runbot upgrade, seems that the CI hook that links in the pull requests the runbot is not working. You can browse them manually through https://runbot.odoo-community.org/runbot (and selecting the repo in the dropdown). For this, I save your time providing the direct link ;):

https://runbot.odoo-community.org/runbot/build/3403398

@tafaRU tafaRU mentioned this pull request Jan 3, 2020
Merged
yajo
yajo approved these changes Jan 3, 2020
View reviewed changes
Copy link
Member

@yajo yajo left a comment

Choose a reason for hiding this comment

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

Cool now. Just one suggestion to consider.

web_responsive/static/src/js/web_responsive.js Outdated Show resolved Hide resolved
@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@Tardo Tardo force-pushed the 12.0-imp-web_responsive-preview_documents branch from 156dd18 to 6ff13e8 Compare January 3, 2020 11:36
@Tardo
Copy link
Member Author

Tardo commented Jan 3, 2020

Thanks @yajo changes done!

@pedrobaeza
Copy link
Member

/ocabot merge major

Now please migrate it to v13

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 12.0-ocabot-merge-pr-1467-by-pedrobaeza-bump-major, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Jan 3, 2020
@OCA-git-bot
Merge PR #1467 into 12.0
c08a76c 
Signed-off-by pedrobaeza
@OCA-git-bot OCA-git-bot merged commit 6ff13e8 into OCA:12.0 Jan 3, 2020
@OCA-git-bot
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yajo yajo yajo approved these changes

@pedrobaeza pedrobaeza pedrobaeza approved these changes

Assignees
No one assigned
Labels
approved merged 🎉 ready to merge
Projects
None yet
Milestone
12.0
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@Tardo @legalsylvain @pedrobaeza @OCA-git-bot @yajo