-
Notifications
You must be signed in to change notification settings - Fork 38.8k
gui: Move static placeholder texts to forms #17702
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
Conversation
There was an issue around the time of Qt 4.6 when placeholder text was introduced, that caused a compile failure when it was specified in the form. As a workaround the placeholder texts were moved to the code. Qt 4 hasn't been relevant to us for ages. So move all (non-parametrized) placeholder texts to the form files instead. It's better to keep this kind of text content together. Makes sure translate/no-translate status is kept as it is.
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
|
Concept ACK. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 67f36e0, I have reviewed the code and it looks OK, I agree it can be merged.
Verified that other objects which use QLineEdit::setPlaceholderText() method have no corresponding *.ui forms.
|
Re-ACK a652dc5, |
fanquake
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK a652dc5 - checked that placeholder text still appears.
|
Typed ACK a652dc5 🚿 Show signature and timestampSignature: Timestamp of file with hash |
a652dc5 qt: Normalize placeholder to avoid using "address book" in sendcoinsentry (Wladimir J. van der Laan) 67f36e0 gui: Move static placeholder texts to forms (Wladimir J. van der Laan) Pull request description: There was an issue around the time of Qt 4.6 when placeholder text was introduced, that caused a compile failure when it was specified in the form. As a workaround the placeholder texts were moved to the code. Qt 4 hasn't been relevant to us for ages. So move all (non-parametrized) placeholder texts to the form files instead. It's better to keep this kind of text content together. Translate/no-translate status is kept as it is. Proof that they still work:     ACKs for top commit: hebasto: Re-ACK a652dc5, `tooltip` and `placeholderText` are identical now. MarcoFalke: ACK a652dc5 🚿 fanquake: ACK a652dc5 - checked that placeholder text still appears. Tree-SHA512: 7d3c1faeef2eb5d4b195d9d78f2a3f161296d869e5059b5e8d308167e3c6c668a3ebabec93dc592762ba15bfc86d51985e20c4e17f1065c8dce84fec036ff5ee
a652dc5 qt: Normalize placeholder to avoid using "address book" in sendcoinsentry (Wladimir J. van der Laan) 67f36e0 gui: Move static placeholder texts to forms (Wladimir J. van der Laan) Pull request description: There was an issue around the time of Qt 4.6 when placeholder text was introduced, that caused a compile failure when it was specified in the form. As a workaround the placeholder texts were moved to the code. Qt 4 hasn't been relevant to us for ages. So move all (non-parametrized) placeholder texts to the form files instead. It's better to keep this kind of text content together. Translate/no-translate status is kept as it is. Proof that they still work:     ACKs for top commit: hebasto: Re-ACK a652dc5, `tooltip` and `placeholderText` are identical now. MarcoFalke: ACK a652dc5 🚿 fanquake: ACK a652dc5 - checked that placeholder text still appears. Tree-SHA512: 7d3c1faeef2eb5d4b195d9d78f2a3f161296d869e5059b5e8d308167e3c6c668a3ebabec93dc592762ba15bfc86d51985e20c4e17f1065c8dce84fec036ff5ee
Summary: > There was an issue around the time of Qt 4.6 when placeholder text was > introduced, that caused a compile failure when it was specified in the > form. As a workaround the placeholder texts were moved to the code. > Qt 4 hasn't been relevant to us for ages. So move all (non-parametrized) > placeholder texts to the form files instead. The change to `openuridialog.cpp/.ui` is not included, because it is parametrized: the placeholder text can be `bitcoincash:` or `bchtest:` or `bchreg:` This is a backport of Core [[bitcoin/bitcoin#17702 | PR17702]] Test Plan: `ninja && src/qt/bitcoin-qt` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D8304
a652dc5 qt: Normalize placeholder to avoid using "address book" in sendcoinsentry (Wladimir J. van der Laan) 67f36e0 gui: Move static placeholder texts to forms (Wladimir J. van der Laan) Pull request description: There was an issue around the time of Qt 4.6 when placeholder text was introduced, that caused a compile failure when it was specified in the form. As a workaround the placeholder texts were moved to the code. Qt 4 hasn't been relevant to us for ages. So move all (non-parametrized) placeholder texts to the form files instead. It's better to keep this kind of text content together. Translate/no-translate status is kept as it is. Proof that they still work:     ACKs for top commit: hebasto: Re-ACK a652dc5, `tooltip` and `placeholderText` are identical now. MarcoFalke: ACK a652dc5 🚿 fanquake: ACK a652dc5 - checked that placeholder text still appears. Tree-SHA512: 7d3c1faeef2eb5d4b195d9d78f2a3f161296d869e5059b5e8d308167e3c6c668a3ebabec93dc592762ba15bfc86d51985e20c4e17f1065c8dce84fec036ff5ee

There was an issue around the time of Qt 4.6 when placeholder text was introduced, that caused a compile failure when it was specified in the form.
As a workaround the placeholder texts were moved to the code.
Qt 4 hasn't been relevant to us for ages. So move all (non-parametrized) placeholder texts to the form files instead.
It's better to keep this kind of text content together. Translate/no-translate status is kept as it is.
Proof that they still work:
