Bug 571572 - UI for setting the location of the toolchain.xml files #114

[G-Ork]

I've added the text field to configure the user toolchains definition. Its just for the convenience.

Other changes:

• toolchain validation based on the maven factories.
• open file in eclipse editor just like the user settings file.
• Set the user toolchains file in MavenExecutionContext.

Sorry i've crushed the previous PR #1639 by lack of git skills :)

Fixes #114

[github-actions]

Test Results

  214 files  ±0    214 suites  ±0   16m 8s ⏱️ -26s
  664 tests ±0    654 ✅ ±0  10 💤 ±0  0 ❌ ±0 
1 328 runs  ±0  1 306 ✅ ±0  22 💤 ±0  0 ❌ ±0 

Results for commit 85aabd2. ± Comparison against base commit 3aa12f5.

♻️ This comment has been updated with latest results.

[HannesWell]

@G-Ork thank you for this contribution.
It looks really good and is a good enhancement for M2E.

I have some remarks below.
In general, especially in the preferences, there is now (and even before) a lot of code-duplication. But this can also be cleaned up in a follow-up change.

[HannesWell]

@G-Ork I have now applied all the requested changes by myself and added a separate first commit where I did the unifications.

With that this is ready for submission.

What would be great, if you could add a test-case in a follow-up PR to verify that it works as expected and is ensured to continue to work in the future.

[G-Ork]

@HannesWell Thank you for for doing the cleanup. I will care for a test the next days.

I have also a strong wish to change the design of that page. I do not like the vertical stacked layout (label, input). I would group in global and user settings and make the layout horizontal (label -> input -> button(s)). I guess we separate this in two PRs?

[HannesWell]

I have also a strong wish to change the design of that page. I do not like the vertical stacked layout (label, input). I would group in global and user settings and make the layout horizontal (label -> input -> button(s)). I guess we separate this in two PRs?

Thanks for working on the test. Yes please split that into two separate PRs.
If you want to re-design the page please also keep in mind that the labels don't have the same length and that the length can even vary (i.e. if the specified file is not available respectively no choice is made, like for the global settings). I'm not sure about the beauty of the page if that varies, but nevertheless I'm interested about your solution. :)

[G-Ork]

Here is a rough draft. I would remove the links for the editor and replace them with buttons. For sake of completeness i would also list the global toolchain. Icons for the buttons instead of the labels would also be more colorful.

Your opinion in advance would be nice to prevent unnecessary work.

[HannesWell]

Your opinion in advance would be nice to prevent unnecessary work.

Looks good in general. But regarding Icons vs Text, it is quite common in the eclipse UI to use texts on buttons instead of icons. For the sake of consistency we should probably keep that.

[mickaelistria]

it is quite common in the eclipse UI to use texts on buttons instead of icons

It's used sometimes in Platform though (eg Project > Properties > Resources has icons for navigate or copy).
I think it's a decent pattern per se, but in such case where there is an icon, we need to think about accessibility and people using a screen reader: if icon doesn't show text directly, it is important to still set a text in hover so screen readers can find something to read.

[G-Ork]

I do not insist on icons. So the next is just my opinion and not serious attempt to argument for icons:

The use of icons and the constants for common icons is not well documented. I guess this this the main reason for the plain designs.

@mickaelistria I think accessibility is not the problem. The button will get tooltip.

[HannesWell]

The use of icons and the constants for common icons is not well documented. I guess this this the main reason for the plain designs.

That could be the case, but I'm not sure.

But I suggest you create a PR from your current changes and we discuss it there. :)