-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Bug 1716806 - Introduces a separator space between radio buttons/checkboxes and their labels/strings #20437
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
base: master
Are you sure you want to change the base?
Conversation
|
Please fix the linting issues, using |
3575144 to
1c571c7
Compare
…kboxes and their labels/strings
1c571c7 to
1700910
Compare
|
Linting has been fixed. Moreover, I've added a test case as requested. The changes include:
The test is ready for XFA test runs. |
|
/botio-linux preview |
From: Bot.io (Linux m4)ReceivedCommand cmd_preview from @timvandermeij received. Current queue size: 0 Live output at: http://54.241.84.105:8877/a06b28735571880/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.241.84.105:8877/a06b28735571880/output.txt Total script time: 1.00 mins Published |
|
/botio test |
From: Bot.io (Windows)ReceivedCommand cmd_test from @timvandermeij received. Current queue size: 0 Live output at: http://54.193.163.58:8877/a7fc82861eab8f0/output.txt |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @timvandermeij received. Current queue size: 0 Live output at: http://54.241.84.105:8877/50f3aad13336564/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/50f3aad13336564/output.txt Total script time: 39.74 mins
Image differences available at: http://54.241.84.105:8877/50f3aad13336564/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/a7fc82861eab8f0/output.txt Total script time: 74.46 mins
Image differences available at: http://54.193.163.58:8877/a7fc82861eab8f0/reftest-analyzer.html#web=eq.log |
|
I don't remember why checkboxes are not rendered in the ref tests... it's a bit painful. |
|
Cool! The 2 approaches were devised after reviewing previous work from other contributor's attempts. They avoid the major pitfall of changing element sizing AFTER layout. To recap:
This PR contains only 1st approach for now, but I could make a hybrid, if needed. The major downside of the 2nd approach is decreasing the size of the entire checkbox or radio, but this could be made subtle if it's adding to the space from the other approach. If the result is satisfactory, I can refactor either code to make it fit nicer in the codebase. For the 1st approach I'm thinking of a helper function for the string manipulation, and for the 2nd I'd parameterize the 2 pixels. |
This PR addresses Bug 1716806. I have devised 2 approaches to tackle this issue, both editing only the
src/core/xfa/template.jsfile:Critically, I have not been able to run
npx gulp xfatestlocally (nor any other test script). Apparently for lack of computational power. So I'm relying on visually inspecting the most critical cases pointed out in the last comment of PR 19765:I suspect the CI automated tests might fail and I'd appreciate some feedback, especially in regards to which of the 2 approaches mentioned above (or neither) seems more promising.