Switch from Semantic-UI to Fomantic-UI?

[migmolrod]

Describe the proposed solution
Switch from Semantic-UI to Fomantic-UI. Or at least study our options.

Fomantic-UI should be a drop-in replacement with maybe some needed modifications in templates if they changed how some components are rendered. The change log can be used for that, starting from 2.4 (which is the first release after the fork, I think):
https://fomantic-ui.com/introduction/new.html#/twofour

Describe alternatives you've considered
I tried to yarn add fomantic-ui-css then use it in my custom gulp file. What happens? You get a huge overhead since Sylius loads Semantic-UI and I load Fomantic-UI in my custom assets. That's just pretty much doubling the size of the assets because reasons.
To avoid that we can directly override the main admin layout stylesheets and javascripts blocks and get rid of the default style.css and app.js files to use custom ones. So we should had to recreate the AdminBundle gulp file inside our custom gulp file. That means we would need to see if Sylius doesn't change any of these files, otherwise we'd need to manually put those changes in our files. A complete pain for maintainability.

Additional context
Semantic-UI development is simply dead. Its owner said:

SUI is used in production for many large apps, and will continue to receive patch/release updates about every month or so at a pace that is reasonable.

That was on 03/19/2018. The sad truth is, last commit in Semantic-UI was on 10/21/2018 (the 2.4.1 release). Which means 8 months with zero pull requests being merged (or even reviewed), bugs not fixed, outdated dependencies (FontAwesome for example) and issues piling up just because having one person as the only able to keep development going in such a big project is simply unfeasible. He's looking for some kind of financing which is completely respectable. But considering how hard that is for Open Source projects, in my honest opinion he should have given some pr reviewing/merging permissions at least to a small group of trustworthy people to help him in the meantime, which he hasn't, unfortunately for us.

Anyway, my opinion about that shouldn't matter. The thing is: in Fomantic-UI we have a better and more active (last commit was three days ago) alternative to Semantic-UI which should serve as a drop-in replacement, so it should not be a big drama to switch over.

[migmolrod]

I have an update. I was discussing this as well in dev-sylius Slack workspace and @vvasiloi pointed me out a better option that can work, at the very least, as a temporal workaround. Thread is here for anyone interested but I'll put below the important information:
https://sylius-devs.slack.com/archives/C3EGDG9LY/p1560938953359000

We can now (apparently) safely override Semantic with Fomantic just with this change in the package.json file:

{
    "dependencies": {
        [...]
        "semantic-ui-css": "npm:fomantic-ui-css@^2.7.0"
    },
    [...]
}

Then yarn install, yarn build and reload page (in my case I had to do a Ctrl+F5 in my browser, Vivaldi, to reload cache as well, to ensure new assets are used). And it works. New icons from the updated version of FontAwesome are working.
I guess new components introduced by Fomantic (toaster, calendar, slider) are unavailable until we manually load them in our custom resources but right now, it's fine for me. Just the updated FontAwesome made my day.

I still think a proper migration to Fomantic-UI is a good long-term goal, to have all Fomantic modules out-of-the-box with Sylius as well as a proper integration with new features introduced by the library.

Edit: please be aware I didn't try every single admin page but the three or four I've tried do work well, nothing seems to be broken by the change.

[kulczy]

Hi @migmolrod to be honest, switching to fomantic isn't our priority. I follow the plans for this project, and big changes are coming, so I’m not sure if this is the best time for change

[migmolrod]

I guess we can stick to 2.x versions for now since they are drop-in replacements. And the good thing is that minor changes are needed for that. It will be still way better than stick to SUI because, even if FUI 2.* becomes "outdated" when FUI 3 comes out, it will be less "outdated" than SUI anyways so I find it a win-win.

The only issue I've found so far with our workaround is this message when yarn build:

semantic-ui-css/components/video is imported by Resources/private/js/shim/shim-semantic-ui.js, but could not be resolved – treating it as an external dependency
semantic-ui-css/components/visit is imported by Resources/private/js/shim/shim-semantic-ui.js, but could not be resolved – treating it as an external dependency
semantic-ui-css/components/colorize is imported by Resources/private/js/shim/shim-semantic-ui.js, but could not be resolved – treating it as an external dependency

I've looked for those components and they are not used anywhere inside Sylius so they can be safely deleted in the shim-semantic-ui.js files (Admin and Shop). And add the new components from FUI 2.x, which are "calendar", "range", "slider" and "toaster" in those same shim-semantic-ui.js files.
If using what Victor suggested ("semantic-ui-css": "npm:fomantic-ui-css@^2.7.0" in the package.json), we don't even need to change "semantic-ui-css" references to "fomantic-ui-css" since the npm alias will take care of that :)

I agree FUI 3 would be quite a pain, tho. And I guess you people in UI are busy enough with the Bootstrap theme, right?

Anyway, as for now, I'm happy enough with the workaround. Even with those errors in yarn build, having to manually import additional FUI modules and all. But if you find my arguments convincing enough and you end up wanting to do this and need help, just tell me. May try to find some time to do a couple PRs if ever needed.

[stale]

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in a week if no further activity occurs. Thank you for your contributions.

[migmolrod]

Ok, let's take this out of the stale. I'll try to clone Sylius, make the changes needed to switch to Fomantic-UI 2.x versions (as it's BC with Semantic-UI used by Sylius), ensure every single Behat test passes, then make the pull request.

Then you decide what to do about it.

[migmolrod]

Ok, I have some questions about this. I'm obviously following this:
https://docs.sylius.com/en/1.6/contributing/code/patches.html
Now my first question comes at

Step 2: Work on your patch

[...]

Choose the right Base Branch

1.4 or 1.5, if you are fixing a bug for an existing feature or want to make a change that falls into the list of acceptable changes in patch versions
master, if you are adding a new feature.

In this case it would be either 1.6 or master (that part in the docs seems outdated) but that's not my question. My questions are:

Do I need to take 1.6 or master as base branch? And if I need to take 1.6 as base branch, how do I do that actually? Because this happens:

migmolrod@debian1:~/projects/sylius$> git checkout -b issue_10463 1.6
fatal: '1.6' is not a commit and a branch 'issue_10463' cannot be created from it

Then if I do git checkout -b issue_10463 master, I understand I'm doing it in the wrong way.
But actually, git branch shows just master, there are no more branches to take as base. And taking a tag (eg, v1.6.0) as base for my branch is also wrong, right?

Also, what will be the relevant CHANGELOG file to be edited in this patch?
Do I need to add [BC BREAK] tag to the relevant change log file?
Will I need to update as well the relevant UPGRADE file? (this only applies if I take master as base branch, right?

[lchrusciel]

Hey @migmolrod,

sorry for the late response. If you would like to open a PR to 1.6 branch you will have to fetch from remote. Assuming, that you have this repository registered as an upstream remote you have to call git fetch upstream and then you should be able to create 1.6 base branch (either by calling git checkout 1.6 or git checkout upstream/1.6 & git checkout -b 1.6

However, your proposal should be open to the master branch ;)

[migmolrod]

Ok, @lchrusciel thanks.
Then I've done it well I think, the PR was done using master AFAIK. I simply followed the "Submitting a patch" article in Sylius documentation.
I first added upstream with:
git remote add upstream git://github.com/Sylius/Sylius.git
Then I created my own branch from master with:
git checkout -b issue_10463 master

As I said, it's the first time I contribute in a project that is big enough to have a "Contribution guide" on its own. I'm not expecting to be successful at first try XD

I forgot, tho, to update the CHANGELOG and the UPGRADE files. Which ones should I edit? The ones referring to version 1.7, I'm guessing.
How can I update my own PR #10738 to upload those changes? I guess I need to make my changes and then fetch upstream, merge upstream master and then rebase my branch from master again, before being able to push it to origin again, right?

[lchrusciel]

Noone made any changes to your branch. Just add another change to the code, commit, and push to your origin. No fetch/merge required, as long as your branch hasn't been merged to the master ;)

[stale]

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in a week if no further activity occurs. Thank you for your contributions.

[vvasiloi]

Do not stale.

[stale]

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in a week if no further activity occurs. Thank you for your contributions.

[vvasiloi]

@migmolrod why did you give up on this? Do you need help?

[migmolrod]

Huge lack of time. Ironically, quarantine has not helped to have more time.
Plus the response to my pull request was "nope, we have no plans for that".
#10738 (comment)

[lchrusciel]

Hey @migmolrod & @vvasiloi,

Just for the sake of clarity, a switch to Formatic-UI is a no-go for us. We would prefer to switch to Webpack + Bootstrap as proposed in https://github.com/Sylius/BootstrapTheme, which is a much better approach. There may be some major challenges on the road, so what you proposed would require a significant amount of time from our side, to ensure proper upgrade path, bc and so on with FormaticUI. Also, it is not on our official roadmap: https://sylius.com/roadmap/

So as stated by @kulczy, we have no plans for that, because we want to provide a better solution for the whole community. However, feel free to lobby this idea, mostly to our Product Owner(@CoderMaggie) or main frontend(@kulczy). Until they are convinced that it is what we should do, we will not invest in this feature. Still, it may be a good idea for some content for blog/documentation.

Anyway thanks for your efforts and pushing topics in our community forward. I hope, that this explanation will be a little bit clearer and you are ok with our decision :)

[vvasiloi]

Thanks for clarification!
I guess the issue can be closed if @migmolrod has nothing to add.