Semantic UI / General Styling

[cheerfulstoic]

I wanted to get this up relatively quickly so that people could take a look and since I won't be able to work on this again probably until Sunday night.

WARNING: Things might be broken because I took away bootstrap when I added Semantic UI. I'm not sure how much depends on bootstrap, so I could use another opinion (particularly on the food selection page which I messed around with, but anything that used bootstrap javascript could very well be broken).

But aside from that, I had some styling for the landing page as well as the food selection page. Everything in general should also look a bit better because I added the segment class to the layout so that there's at least a white background behind the text

[cheerfulstoic]

I'd also like to only include the Semantic UI files which are being used to reduce the payload for loading the page, but this should work for getting this rolling. I also am not sure I'm doing the right thing to load Semantic UI (it seems like maybe I should be using yarn?)

[komizutama]

@cheerfulstoic I'd imagine probably most of the bootstrap issues would be related to the exadmin console. though tbh, I'm not seeing any. I know there were a few options when @bglusman set up the initial project. And subsequently, I did a test project setup of my own and swapped in material design without really any hitches.

It looks like the Wallaby test failures are related maybe to how the factories are generating credit types in testing, maybe? Specifically two visible links and a css class {credit_type_name-0, credit_type_name-13, .credit-type-name-18} @bglusman do you have thoughts on that?

If @bglusman doesn't mind, I'm going to merge this as it already looks a lot better than the prior version. Thank you!

[bglusman]

Don't really want to merge with failing tests.... that's kind of a no-no!

[bglusman]

The food selection page looks completely broken to me, and there are multiple JS errrors happening from removal of bootstrap.js which the tabs on food selection rely on. Maybe we can drop the boostrap css but leave some of the js? or... I dunno, maybe there's equivalent tab functions in semantic ui.js?

[bglusman]

@cheerfulstoic have you looked at the food_selection page locally? it's kind of the entire focus of the app, though @komizutama has a branch that was trying to redesign it completely. I think starting a new parallel page with the new desired look and feel will be more productive and less conflict-prone, so maybe the two of you can collaborate on that?

[bglusman]

Oh, maybe we're just missing setup for tests? I hadn't run yarn locally, though there is some weird behavior ehre and there... try and look more later!

[cheerfulstoic]

Absolutely I want to get the tests passing first, sorry about that! When I tried to sit down and fix them I got the tests working, but then the development environment wasn't working and I went down a bit of a rabbit hole of trying to re-build the docker images. If that doesn't work I might just try running it on my machine for now.

If we want to get the style back in order, I could add the bootstrap js file back for now and see if that works.

I was able to replace certain tabs with semantic UI's tab UI, so I could also replace the other tabs that I didn't already do, though that could be a secondary priority if it's OK to have bootstrap and semantic UI at the same time.

I think I jumped in a bit quickly without completely understanding how the UX is supposed to work. I can use the demo, but honestly I couldn't do anything in the demo anyway (am I supposed to be able to click on things? Do I need to be logged in? There's a Log Out button, but I didn't log in).

I'm happy to look at the parallel branch too if that's the desired path anyway.

[bglusman]

Oh man totally this is all on us but thanks for all the work so far! The demo is a guest account so it's "logged in" as a guest user who had no credits.. locally there's an admin user and we can make you an account on production and or staging... The UI is imperfect but yes understanding a bit first may help! Apologies for confusion and any extra work!

[bglusman]

Oh also the manage users page had links to log in as users that seem to be gone now? generated urls with a token? I'll look at what changed and maybe fix up but FYI that was how youb logged in as a guest, and there's "Guest" and guest role... too confusing/need to rename

[bglusman]

We can also by the way have multiple layouts, a legacy bootstrap layout and a new semantic layout, each controller controls what layout it renders pages in so if it helps at all can just fork the app layout and leave food selection page using old layout but have other stuff use new one, and then use new layout to build new replacement food selection page in semantic with Thia's grid design. I think it will be way easier to do that from a clean slate than to rework the current table design into it gradually, but I'm not the front end expert you all are ;-)

[cheerfulstoic]

Ok!

I added the link for logging in as a user. I actually found it in templates/user_selection/index.html.eex so I copied it to templates/food_selection/index.html.eex (including to the user_view.ex). That could perhaps be DRYed later (I added a comment to remind about this). I'm not completely sure the link is working, though, because the user doesn't show up in the menu...

I fixed the tests. They all revolved around adding attributes to the tab links, but I don't think they actually affected functionality, so there might be some brittle tests there (?)

I also upgraded bcrypt_elixir from 1.0.4 to 1.0.5 because I was getting an error when trying to set things up (I gave up on using Docker for now and am just using homebrew).

I'm not seeing JS errors on the food selection page without the bootstrap JS, but I might not be logged into it correctly (or maybe I've fixed something since then?). Are you still seeing those?

Regarding the language selection, perhaps we could leave it in for now and remove it in another PR so as not to distract from all of the other stuff that's going on here.

[bglusman]

I think can just use defdelegate here, or possibly import the function from the UserSelectionView, assumign this is actually needed... I'm a little fuzzy on why this is needed here now, or if still needed there, but just FYI

[cheerfulstoic]

I figured out the issue with the distribution panel on the right hand side! When I included semantic UI I used the syntax <script /> but I guess with script tags you really do need to do <script></script>. It was causing the app.js file to not load after it.

So that's fixed and I replaced the style with a bit of Semantic UI. Please have a look!

[cheerfulstoic]

Checking in to see if you have any questions. ;) . In the last commit I left the bootstrap.js, though I think things might still work without it. But for the sake of getting things both working and pretty I thought it would be good to leave it in for now. It would be enough to take it out and test it separately.

[bglusman]

Thank you for checking in! I apologize, I left my computer that runs open pantry at with Friday and never got it running on my windows machine at home, I briefly ran the other day and it looked basically good but I forget if the functionality was still working properly adding things to the cart and checking out? If that all still works I'm good to merge! If not, prefer to fix it first. Our integration used to test that properly but the I've that does is pending since last redesign, be nice to get fixed!! If you're sure that works I'll merge, if not sure I'll try and check tomorrow!

[bglusman]

Oh also a little preference is we can fire making semantic UI a yarn dependency instead of vendoring if possible? I'd wanted to do same thing with bootstrap, I forget why it wasn't done, but maybe one of us can take a crack at that if possible first? Makes diff gigantic!

[cheerfulstoic]

The "add to cart" is the thing that I saw that wasn't working but then started working after my fix, so we should be good there. I'll add semantic UI as a yarn dependency (tonight if I have time). Thanks!

[bglusman]

Awesome. Yeah if that works (asking with removing from vendor and rebasing the noise out of history), I'm good! If I can help to review or debug anything I'll try!

[bglusman]

OK can confirm this branch is working locally! And much of it looks improved, though the food selection is in a funny transitional state for sure, functional but probably a lot less usable than it was before... also there's merge conflicts as shown above. Hugely appreciate all your work here @cheerfulstoic ! depending on your time and thoughts, would like to a) resolve conflicts, b) take a shot at making the semantic ui a non-vendor dependency, and c) perhaps if not TOO hard, leave the food selection page more or less as it was/on a parallel page maybe? Like maybe we can use all the work to get this one going as one route/page and also keep the old one in place with bootstrap styling, so we can iterate on and improve the new version without breaking the old one? We'd still like to continue user testing if possible with Masbia and other orgs, but I'm waiting on @komizutama to set up a meeting with me and Alex at Masbia to figure out next steps, and being able to have a decent deploy strategy where we keep latest stuff going out but we can also not be blocked from making dramatic changes to improve things seems important going forward. I can take a crack at doing that from this branch if you prefer @cheerfulstoic or you can, or some of both? I plan to spend some time this coming weekend on it, and maybe one or two nights this week. Either way, thanks again so much and can't wait to get this in and get started on next steps!

[bglusman]

(PS - happy to give you commit access to the repo and let us both work on a common integration branch if it helps, since right now you can't push here and I can't push to your fork, but we'll deal with that later...)

[cheerfulstoic]

Thanks, I've was struggling with a weird error for a while and realized I had forgotten to set GUARDIAN_SECRET_KEY. Was thinking of (later) putting in something which raises an exception on startup if that's not set (so that somebody else doesn't need to figure it out).

I'm trying to get brunch to use the semantic-ui files that yarn generated. Once I do that I think I should be good (though off to bed now...)

[bglusman]

@cheerfulstoic thanks so much for getting the yarn stuff figured out! And yeah, we have an open issue about automating/fixing the dependecy on GUARDIAN_SECRET_KEY, it should just have a good fall back value in dev, its only needed to be a true env var for production security. Apologies.

There's a conflict now, plus the diff still has the manually added files in there which makes diff and history super noisy, but lets squash/rebase this up while removing those and then I'm excited to merge this in! Maybe we'll fork the food selection page after that, I dunno, but don't want to block further work.

[cheerfulstoic]

I have a small bit of code that checks the GUARDIAN_SECRET_KEY, though to keep from piling on this PR I'm going to leave it for later.

I'm not sure what's up with the conflict. I'm looking at it now, but I've tried merging the upstream master to my master and then merging my master to my branch but it all seems up-to-date. I'll keep looking at it. It's saying that if I had write access I could resolve the conflict in GitHub, so that might be easiest...

Regarding the files, they're definitely still there but they were actually added with yarn, which is awesome. The problem that I was just coming online to fix is where they should go. They need to exist in a place which is in the .gitignore, I think, so that they can get install in development and production without making a lot of files in the repo, so I think I'm going to add an assets/vendor/yarn directory which will be ignored and which Semantic UI will be installed into whenever yarn install is run.

Part of the reason it took me a bit of figuring on how to get Semantic UI to work with Yarn BTW: (and there's a long issue chain out there on this already) Semantic UI uses Gulp to ask users questions and Yarn doesn't allow interactivity, so you need to specify "autoInstall": true in the semantic.json file.

[cheerfulstoic]

I think I've got the yarn situation in order. If you don't like the directory or something else I can certainly adjust it.

I'm still not sure what's up with the conflicts, though... Could you either give me write access or let me know which files are involved in the conflicts? If you have any questions about resolving the conflicts I'd be happy to help figure it out...

[bglusman]

@cheerfulstoic hey! Just pushed the branch here to possibly resolve myself, but also happy to give you write access! For the directory, shouldn't it just go in the node_modules directory that's already gitignored like all the other dependencies? Just curious to understand if there's a special reason it needs to be somewhere else....

[bglusman]

@cheerfulstoic ugh I'm sorry that same interactive semnatic UI issue you mentioned is breaking travis now... I bet we can modify the command travis uses on this branch to pipe the command through unix yes or something, if you want I can try and fix that but don't want to step on your toes/work on branch while you'r editing also, so let me know if I should take a crack at fixing it on this branch or if you want to try first.

[bglusman]

this will probably help Semantic-Org/Semantic-UI#1816 linked from Semantic-Org/Semantic-UI#1824

[bglusman]

Oh whoops! Did I resolve that conflict wrong? Sorry!

[cheerfulstoic]

No, I don't think so (I'm still not really sure what the conflict was...). I think the extra body { that I just removed was in my changes from a little while ago, but I'm not sure how Travis would have passed up until now. Waiting on Travis to run again (also, I'm on Slack now)