Community
Participate
Working Groups
We need to provide a Globalization Preferences dialog to set the following properties: - Layout direction of the GUI - Bidi preferences : bidiSupport flag and textDirection - Calendar type.
Created attachment 238600 [details] The New Globalization preference dialog Hello Markus, Could you please review this patch for the new Globalization preference dialog Please note that we also need to add the layout direction and calendar type preferences. But I would like to have your feedback before completing the work Thank you very much for your review Best Regards
Created attachment 238601 [details] Adding the new dialog to the General Preferences
Just a couple of words on the background and motivation. We are not adding any new preferences which have not existed before. All we do is to add a convenient way to control globalization (or Bidi) preferences for the end user. At the moment those preferences are controlled via command line. The rational for that is as follows: 1. A lot of end user facing components based on Eclipse don't expect any command line level configuration. Namely end users are not expected to access the command line. 2. Globalization / Bidi proprieties in the preferences are associated with display aspect of the product which by its nature is very individual and thus is expected to be controlled by the end user. Configuration via GUI is considered much more convenient. 3. Finally, we got explicit requests from products adopting Bidi feature in the platform (such as control over base text direction and support for structured text) to provide explicit GUI level control over those preferences.
Created attachment 238602 [details] Sample UI for control over g11n prefs.
Command line options responsible for invocation of bidi support and specification of base text direction value were introduced via bug 307307 . API for control over base text direction currently guided by command line options were introduced via bug 402256 .
Specification of calendar type was available as part of providing support for Unicode locale extensions specification in the command line. For more details please see bug 243270 .
>We are not adding any new preferences which have not existed before. The command line switches ensured that the whole application runs with the same constant settings. The preferences are something new, because these settings were typically not changed dynamically before. IMO, we can add the preference page if the "You may have to restart" note is acceptable. Preferences should be initialized explicitly in WorkbenchPreferenceInitializer#initializeDefaultPreferences(). Open a call hierarchy on other IPreferenceConstants fields to see that. UI problems: - needs mnemonics - needs colon after "Text direction" - Combo should be on the same line as "Text direction:" - Combo should not take full width - We don't use the term "widget" in the UI, and a widget can't be restarted. Better don't name an entity at all and say e.g.: "A restart is required for these preferences to take full effect." - Why is "Default" in the Combo? Do we need that option at all? Without further explanations, the user can't know what that "default" means. Is it the OS default? Or the default for the OS locale? Or the default for the Eclipse locale? - Is "Bidi" a common term for end users? Shouldn't this be a bit more verbose?
"You may have to restart" note is acceptable. Bidi is a short form of bidirectional. So we will change: Bidi preferences to "Bidirectional support preferences" Enable Bidi support to "Enable bidirectional support"
Created attachment 238959 [details] The new Globalization preferences dialog
(In reply to Markus Keller from comment #7) > >We are not adding any new preferences which have not existed before. > > The command line switches ensured that the whole application runs with the > same constant settings. The preferences are something new, because these > settings were typically not changed dynamically before. > > IMO, we can add the preference page if the "You may have to restart" note is > acceptable. > > Preferences should be initialized explicitly in > WorkbenchPreferenceInitializer#initializeDefaultPreferences(). Open a call > hierarchy on other IPreferenceConstants fields to see that. > Done > UI problems: > - needs mnemonics I put mnemonics on both Enable Bidi support checkbox and Text Direction combo > - needs colon after "Text direction" Done > - Combo should be on the same line as "Text direction:" Done > - Combo should not take full width Done > - We don't use the term "widget" in the UI, and a widget can't be restarted. > Better don't name an entity at all and say e.g.: "A restart is required for > these preferences to take full effect." Done > - Why is "Default" in the Combo? Do we need that option at all? Without > further explanations, the user can't know what that "default" means. Is it > the OS default? Or the default for the OS locale? Or the default for the > Eclipse locale? It is OS default. I updated the label > - Is "Bidi" a common term for end users? Shouldn't this be a bit more > verbose? I changed Bidi preferences to "Bidirectional support preferences" Enable Bidi support to "Enable bidirectional support" according to Tomer's suggestion
Hello Markus, I addressed your commnents Can you please review the changes? Thank you Best Regards
Created attachment 238963 [details] Screenshot for the G11N Preferences dialog
OK, please finish the dialog. I won't test/release several iterations. If you don't plan to add the other options mentioned in comment 0, then you have to remove the Group around the options. IPreferenceConstants.TEXT_DIRECTION should correctly document supported values, and the default should not be set to an unsupported value. Make a pass over the Javadocs and fix wrong information and incorrect language. What's the take if the old command line arguments are supplied? Do they override the stored settings, or do they set the defaults?
Created attachment 239021 [details] The new Globalization preferences dialog
(In reply to Markus Keller from comment #13) > OK, please finish the dialog. I won't test/release several iterations. > > If you don't plan to add the other options mentioned in comment 0, then you > have to remove the Group around the options. > > IPreferenceConstants.TEXT_DIRECTION should correctly document supported > values, and the default should not be set to an unsupported value. IPreferenceConstants.TEXT_DIRECTION may have the following values: "ltr", "rtl","auto" and "". I documented this in the IPreferenceConstants source > > Make a pass over the Javadocs and fix wrong information and incorrect > language. > > What's the take if the old command line arguments are supplied? Do they > override the stored settings, or do they set the defaults? if user specified values in both command line and Preferences G11N dialog the G11N dialog has the precedence. Of course if default values are specified in the G11N pref dialog then we take the command line options (if any)
Hello Markus, I think I finished the dialog. Can you please review the changes? Thank you Best Regards
Created attachment 239022 [details] Capture screen for the G11N Preferences dialog
(In reply to Moshe WAJNBERG from comment #17) > Created attachment 239022 [details] > Capture screen for the G11N Preferences dialog Please clean up the widget arrangements. Indent dependent options. Align the combos. See existing Eclipse preference pages like Java > Editor > Content Assist for ideas. Is the group really necessary? It looks strange to have one option outside a group and the others in a group. Is there really no restart required to activate changes in Unicode locale extensions? I don't know those APIs are typically used, but I guess some widgets also need to be re-created to take the new options into account.
Created attachment 239188 [details] The new Globalization preferences dialog
Created attachment 239189 [details] Screenshot for the G11N Preferences dialog
(In reply to Markus Keller from comment #18) > (In reply to Moshe WAJNBERG from comment #17) > > Created attachment 239022 [details] > > Capture screen for the G11N Preferences dialog > > Please clean up the widget arrangements. Indent dependent options. Align the > combos. See existing Eclipse preference pages like Java > Editor > Content > Assist for ideas. I indented the dependent options and aligned the combos (i attached a screenshot) > > Is the group really necessary? It looks strange to have one option outside a > group and the others in a group. > > Is there really no restart required to activate changes in Unicode locale > extensions? I don't know those APIs are typically used, but I guess some > widgets also need to be re-created to take the new options into account. Yes you are right. I made the note applicable for all the G11N preferences
Hello Markus, I think I addressed your comments Can you please review the changes? Thank you Best Regards
> Is the group really necessary? You didn't answer this question, and I still think the group should be removed. What has the graphical layout direction to do with bidi? It's only one direction. The right-aligned combos are strange when the dialog happens to be wider than usual. You should put all these controls into a single GridLayout with two columns. The text field can be full-width, but the combos best go with their default width and left-aligned. Hard-coded pixel widths are not a good solution. To separate the three main options vertically, you may want to add an empty row in between (e.g. put a Label without text there).
(In reply to Markus Keller from comment #23) > I still think the group should be removed. We don't mind to remove it if this is so important. The idea was to group together all Bidi unique options and separate them from the rest of globalization options. > What has the graphical layout direction to do with bidi? > It's only one direction. "Graphical layout direction" is equvalent to dir attribute. When dir = LTR the UI flow is from left to right (not mirrored screen) When dir = RTL the UI flow is from right to left (mirrored screen) This is VERY unique bidi property which controlls if UI appears flipped or not.
Created attachment 239558 [details] The new Globalization Preferences dialog
Created attachment 239559 [details] Screenshot for the G11N Prefs dialog
(In reply to Markus Keller from comment #23) > > Is the group really necessary? > > You didn't answer this question, and I still think the group should be > removed. What has the graphical layout direction to do with bidi? It's only > one direction. OK. I removed the group. > The right-aligned combos are strange when the dialog happens to be wider > than usual. > > You should put all these controls into a single GridLayout with two columns. > The text field can be full-width, but the combos best go with their default > width and left-aligned. Hard-coded pixel widths are not a good solution. > > To separate the three main options vertically, you may want to add an empty > row in between (e.g. put a Label without text there). OK. That's what I have done
Hello Markus, I think I addressed all your comments Can you please review the changes? Thank you Best Regards
Hello Markus, Any updates ? Thank you
Created attachment 240045 [details] cleansed fix The text field and combos were still not aligned as they should be. The two bidi-related options didn't work at all. The dialog page called BidiUtils.setTextDirection(""), which threw IAEs, and the initialization of the graphical layout direction in Workbench.createAndRunWorkbench(...).new Runnable() {...}.run() was too late to be picked up by the main workbench window. I've moved this into WorkbenchPlugin#getDefaultOrientation(). I've also added preference keywords. Moshe, please provide a help page for this preference page and register it in /org.eclipse.platform.doc.user/contexts_Workbench.xml like this: <context id="globalization_preference_page_context"> <description>Allows you to configure Bidi and other globalization preferences.</description> <topic label="Preferences - Globalization" href="reference/ref-globalizationprefs.htm"/> </context> I've already set this context ID in the GlobalizationPreferencePage in the patch. Moshe, please test all options on the preference page again. And you have to sign the https://wiki.eclipse.org/CLA (can't commit anything without that).
Created attachment 240066 [details] Globalization Preferences dialog Help
(In reply to Markus Keller from comment #30) > Created attachment 240045 [details] > cleansed fix > > The text field and combos were still not aligned as they should be. > > The two bidi-related options didn't work at all. The dialog page called > BidiUtils.setTextDirection(""), which threw IAEs, and the initialization of > the graphical layout direction in Workbench.createAndRunWorkbench(...).new > Runnable() {...}.run() was too late to be picked up by the main workbench > window. > > I've moved this into WorkbenchPlugin#getDefaultOrientation(). > > I've also added preference keywords. > > > Moshe, please provide a help page for this preference page and register it > in /org.eclipse.platform.doc.user/contexts_Workbench.xml like this: > > <context id="globalization_preference_page_context"> > <description>Allows you to configure Bidi and other globalization > preferences.</description> > <topic label="Preferences - Globalization" > href="reference/ref-globalizationprefs.htm"/> > </context> > > I've already set this context ID in the GlobalizationPreferencePage in the > patch. I attached a patch for the help page. > > Moshe, please test all options on the preference page again. > Your changes look good. Thanks > And you have to sign the https://wiki.eclipse.org/CLA (can't commit anything > without that). I commited to CLA
(In reply to Markus Keller from comment #30) > Created attachment 240045 [details] [diff] > cleansed fix Released as http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=cd159efd28799e71212681b63bc74f016fb538b9 (In reply to Moshe WAJNBERG from comment #31) > Created attachment 240066 [details] [diff] > Globalization Preferences dialog Help Released with quite a few fixes as http://git.eclipse.org/c/platform/eclipse.platform.common.git/commit/?id=20c7376e4580defd673c0ff1854f38a70777e00e
Look like this causes bug 428977. Probably accessing preferences from within the WorkbenchPlugin causes the workspace to be set in OSGi. PW
(In reply to Paul Webster from comment #34) Sorry about that. Fixed with http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=416e5203747845a5938c2c195941f3e2c19b94ac for bug 428977. The workbench window still comes up as RTL if the workspace preference is set like that. The workspace chooser dialog obviously can't respect a workspace setting, since the workspace is not yet known. If a product wants to set the workspace chooser to RTL, then it has to use the command line option -dir rtl .
Verified on the 05/05 build Thank you very much Markus for your help
