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

[14.0][IMP] shopfloor_delivery_shipment_mobile: have long package name fit on one line #688

Merged
merged 2 commits into from
Nov 22, 2023
Merged

[14.0][IMP] shopfloor_delivery_shipment_mobile: have long package name fit on one line #688

merged 2 commits into from
Nov 22, 2023

Conversation

TDu
Copy link
Member

@TDu TDu commented Jul 26, 2023 •
edited
Loading

ref.: rau-180

@OCA-git-bot
Copy link
Contributor

Hi @simahawk,
some modules you are maintaining are being modified, check this out!

@TDu TDu changed the title [14.0 [14.0][IMP] shopfloor_delivery_shipment_mobile: have long package name fit on one line Jul 26, 2023
simahawk
simahawk reviewed Jul 28, 2023
View reviewed changes
Copy link
Contributor

@simahawk simahawk left a comment

Choose a reason for hiding this comment

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

I guess sh_ in commit msg was meant to be sf_?

shopfloor_mobile_base/static/wms/src/css/main.css
@@ -457,6 +457,10 @@ main.v-content > .v-content__wrap > .header .container {
line-height: 1.1rem;
}

.v-card .v-card__title-smaller {
font-size: 0.9rem;
Copy link
Contributor

Choose a reason for hiding this comment

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

this does not ensure the name fits... no?

Copy link
Member Author

Choose a reason for hiding this comment

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

No but it is more likely. Maybe the name of the pr should be changed ?

Copy link
Contributor

Choose a reason for hiding this comment

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

@TDu Can you try with something similar to this exemple: https://jsfiddle.net/hymbof7o/

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 did not know about the vw unit. It stands for View Width (1vw is 1% of the width of the viewport). That will not help.
As for the whitespace: nowrap; it will not wrap the text to the next line, but it will not dynamically resize the content to fit.

Copy link
Contributor

Choose a reason for hiding this comment

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

Have we considered @container?
https://developer.mozilla.org/en-US/docs/Web/CSS/@container

JuMiSanAr
JuMiSanAr approved these changes Sep 5, 2023
View reviewed changes
shopfloor_mobile_base/static/wms/src/css/main.css
@@ -457,6 +457,10 @@ main.v-content > .v-content__wrap > .header .container {
line-height: 1.1rem;
}

.v-card .v-card__title-smaller {
font-size: 0.9rem;
Copy link
Contributor

Choose a reason for hiding this comment

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

Have we considered @container?
https://developer.mozilla.org/en-US/docs/Web/CSS/@container

@jbaudoux
Copy link
Contributor

@TDu Fix commit messages. Replace sh__ by sf_. Drop a comment to say that we want to have the transporter package name fitting one line on small mobile screen.
Once done, we can merge

@TDu TDu force-pushed the 14-sh-shipment-pack-name branch from 71a060a to 8e21cdc Compare November 15, 2023 10:44
@TDu
Copy link
Member Author

TDu commented Nov 15, 2023

@jbaudoux I added a comment and fix the commit message

@jbaudoux
Copy link
Contributor

/ocabot merge minor

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 14.0-ocabot-merge-pr-688-by-jbaudoux-bump-minor, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Nov 15, 2023
@OCA-git-bot
Merge PR #688 into 14.0
2793b3d 
Signed-off-by jbaudoux
@OCA-git-bot
Copy link
Contributor

@jbaudoux your merge command was aborted due to failed check(s), which you can inspect on this commit of 14.0-ocabot-merge-pr-688-by-jbaudoux-bump-minor.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

@jbaudoux
Copy link
Contributor

/ocabot rebase

@OCA-git-bot
Copy link
Contributor

@jbaudoux The rebase process failed, because command git push --force camptocamp tmp-pr-688:14-sh-shipment-pack-name failed with output:

remote: Permission to camptocamp/wms.git denied to OCA-git-bot.
fatal: unable to access 'https://github.com/camptocamp/wms/': The requested URL returned error: 403

@jbaudoux
Copy link
Contributor

@TDu Can you rebase?

TDu added 2 commits November 22, 2023 08:36
@TDu
sf_mobile_base: improve detail card title cusomization
e73cd0f 
Allows to set a class on the detail card title.
And one is added to the css to have smaller font.
@TDu
sf_delivery_shipment_mobile: make package name smaller
61fa0b1
@TDu TDu force-pushed the 14-sh-shipment-pack-name branch from 8e21cdc to 61fa0b1 Compare November 22, 2023 07:37
@TDu
Copy link
Member Author

TDu commented Nov 22, 2023

@TDu Can you rebase?

Done @jbaudoux

@jbaudoux
Copy link
Contributor

/ocabot merge minor

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 14.0-ocabot-merge-pr-688-by-jbaudoux-bump-minor, awaiting test results.

@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). 🤖

@OCA-git-bot OCA-git-bot merged commit f6e03ba into OCA:14.0 Nov 22, 2023
6 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 48894cf. 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

@simahawk simahawk simahawk left review comments

@JuMiSanAr JuMiSanAr JuMiSanAr approved these changes

@jbaudoux jbaudoux jbaudoux approved these changes

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

Successfully merging this pull request may close these issues.
5 participants
@TDu @OCA-git-bot @jbaudoux @simahawk @JuMiSanAr