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

[10.0] l10n_nl_partner_name improvements #128

Merged

Conversation

hbrunn
Copy link
Member

@hbrunn hbrunn commented Dec 11, 2017

this started with wanting to have the form look like

screenshot_20171211_124726

the rest are fixes I came across on the way. I'm of a mind to propose a similar patch for partner_firstname, but that's slightly more tricky because there, we've got to cope with different name orders.

Another thing I've in mind (but nobody paying for it) is to make infix and prefix configurable, possibly on group level. Then users can decide if the want to use only infixes, only prefixes or both. The forms then can be adapted with appropriate group specific views

depends on OCA/server-tools#1090

@hbrunn hbrunn added this to the 10.0 milestone Dec 11, 2017
@hbrunn hbrunn force-pushed the 10.0-l10n_nl_partner_name-improvements branch from ad1977e to 0b37538 Compare December 11, 2017 12:08
@hbrunn
Copy link
Member Author

hbrunn commented Dec 11, 2017

PS: Contrary to the screenshot, I swapped first name and initials after talking to my colleagues

@hbrunn hbrunn force-pushed the 10.0-l10n_nl_partner_name-improvements branch from f63c09b to 56964c6 Compare December 11, 2017 14:47
@NL66278
Copy link
Contributor

NL66278 commented Jan 26, 2018

schermafdruk van 2018-01-27 00-06-01

Maybe it is our particular installation (and we should definitely move the gender field). Still we get all the name fields except firstname very small and confined to less then half of the screen.

@hbrunn
Copy link
Member Author

hbrunn commented Jan 29, 2018

@NL66278 looks like there's still a button box or an oe_left or oe_right active on your form

@lfreeke
Copy link
Contributor

lfreeke commented Feb 19, 2018

@hbrunn The initials and infix field are very small. You hardly can see what you type. Should we add some more width to the fields?

@NL66278
Copy link
Contributor

NL66278 commented May 11, 2018

@hbrunn Could you please rebase this one? We have a merge conflict now.

@hbrunn hbrunn force-pushed the 10.0-l10n_nl_partner_name-improvements branch from 56964c6 to 7ad2ae6 Compare May 14, 2018 09:16
@hbrunn hbrunn force-pushed the 10.0-l10n_nl_partner_name-improvements branch from 7ad2ae6 to 4751bc3 Compare May 14, 2018 09:19
@hbrunn
Copy link
Member Author

hbrunn commented May 14, 2018

@NL66278 done. Please add your formal review so that we're a step closer to merge this in order to avoid similar problems in the future.
I assumed a 'normal' initial and infix to be usually two characters wide, what would be better values in order to fix @lfreeke's comment?

@RoelAdriaans
Copy link

Maybe bump the version, because this pull requests changes views, that can break inherited views?

@hbrunn hbrunn force-pushed the 10.0-l10n_nl_partner_name-improvements branch from 4751bc3 to 8d66a52 Compare May 15, 2018 18:27
Copy link
Contributor

@NL66278 NL66278 left a comment

Choose a reason for hiding this comment

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

👍 Code review and test

@NL66278
Copy link
Contributor

NL66278 commented Jun 15, 2018

@StefanRijnhart This no longer needs fixing (has been done), but I cannot remove the label.

@CasVissers-360ERP
Copy link

@StefanRijnhart LGTM

Only possible improvement can be to increase infix to 7 characters (van der)

Copy link

@CasVissers-360ERP CasVissers-360ERP 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 (in v11)

@StefanRijnhart StefanRijnhart merged commit ebb261f into OCA:10.0 Jul 19, 2018
@CasVissers-360ERP
Copy link

@hbrunn Only possible improvement can be to increase infix to 7 characters (van der)
Can you decide on this?

daramousk pushed a commit to daramousk/l10n-netherlands that referenced this pull request Nov 21, 2018
* [DEL] obsolete manifest

* [IMP] allow to fill in names in a more natural way
daramousk pushed a commit to daramousk/l10n-netherlands that referenced this pull request Nov 30, 2018
* [DEL] obsolete manifest

* [IMP] allow to fill in names in a more natural way
daramousk pushed a commit to daramousk/l10n-netherlands that referenced this pull request Dec 12, 2018
* [DEL] obsolete manifest

* [IMP] allow to fill in names in a more natural way
daramousk pushed a commit to daramousk/l10n-netherlands that referenced this pull request Jan 17, 2019
* [DEL] obsolete manifest

* [IMP] allow to fill in names in a more natural way
hbrunn added a commit that referenced this pull request Jan 28, 2019
* [DEL] obsolete manifest

* [IMP] allow to fill in names in a more natural way
hbrunn added a commit to hbrunn/l10n-netherlands that referenced this pull request Jan 12, 2020
* [DEL] obsolete manifest

* [IMP] allow to fill in names in a more natural way
NL66278 pushed a commit to NL66278/l10n-netherlands that referenced this pull request Jan 13, 2022
* [DEL] obsolete manifest

* [IMP] allow to fill in names in a more natural way
ntsirintanis pushed a commit to sunflowerit/l10n-netherlands that referenced this pull request Mar 25, 2024
* [DEL] obsolete manifest

* [IMP] allow to fill in names in a more natural way
hbrunn added a commit to hbrunn/l10n-netherlands that referenced this pull request Jul 22, 2024
* [DEL] obsolete manifest

* [IMP] allow to fill in names in a more natural way
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.

6 participants