Bugs:
1. Original value display does not contain any newlines when needed. This may invalidate using the description field for the original value. We may need someone with more complex forms usability expertise to help devise a better UI here.
2. Original value display uses the same language as the translation, so it is in fact the value of the translation before any changes on the page. That is not as intended, it should be the original config value. This is due to the config access using the same context as the form itself.
Comment | File | Size | Author |
---|---|---|---|
#19 | 1945184-original-value-display-19.patch | 3.13 KB | vijaycs85 |
#19 | 1945184-original-value-display-19-test-only.patch | 2.22 KB | vijaycs85 |
#19 | 1945184-diff-17-19.txt | 708 bytes | vijaycs85 |
#17 | 1945184-original-value-display-17.patch | 3.13 KB | vijaycs85 |
#17 | 1945184-original-value-display-17-test-only.patch | 2.21 KB | vijaycs85 |
Comments
Comment #1
vijaycs85Patch to fix both issue #1 and #2 in summary.
Comment #2
Gábor HojtsyHow does this solve the problem? The form has values and source intertwined, so I'd assumed entering / leaving contexts constantly would be what is needed here, or keeping a previously retrieved $config around, if that would work. Just putting the context entering later sounds like would not actually solve this? How does it do it?
This trailing comma should be no issue, it should even be there according to the coding standards.
Comment #3
vijaycs85Thanks for the review @Gábor Hojtsy. I have updated to get the base_config before form builder and added comma for $description.
Comment #4
Gábor HojtsyStarting to look good :)
It would be great to add a comment here on why is this being done. :)
"keyed by configuration names" would be a good addition to the docs.
Comment #5
vijaycs85Updating doc changes in #4.
Comment #6
Gábor HojtsySuperb, thanks, let's get this committed :)
Comment #7
vijaycs85Committed and pushed to 8.x-1.x. Thanks!!!
Comment #8
Gábor HojtsySuperb, thanks!
Comment #10
Gábor HojtsyAs per YesCT, this is not yet fixed. We'll also need tests for this.
Comment #11
vijaycs85Updating fix with test.
Comment #12
Gábor HojtsyGood catch on the issue where the config page is visited in another language :)
Comment #13
Gábor HojtsyTest only should fail :D
Comment #14
vijaycs85Sure @Gábor Hojtsy. Here it will fail :)
Comment #16
vijaycs85updating URL...
Comment #17
vijaycs85Minor URL fix...
Comment #18
Gábor HojtsyTest-only patch should still fail :D I think I found the reason it does not.
This just visits the French translation editing page, but the page is not in French. That would need a path prefix of 'fr'. $translation_base_url starts with 'admin/...'.
So drupalget('fr/' . $translation_base_url . '/edit/fr') I think :)
Comment #19
vijaycs85yeah, my bad...
Comment #21
vijaycs85This is ready for review now.
Comment #22
Gábor HojtsyThanks! Committed the fix with updated code comments. It is not the base context but the override free context + other updates to the comments around contexts to make this easier to understand. Woot!
http://drupalcode.org/project/config_translation.git/commitdiff/8a09f89f...