Reimplement the Navbar using Elm Bootstrap #3
Conversation
Previously they were tagged with Page.Other, but the Page type has tags for Login and Register, so why not use them. The effect is that now navigating to these pages highlights the appropriate link in the nav bar.
|
Trouble is, that the |
This makes more sense. The whole page module is really about framing the content between header and footer. The union type previously called Page is not representing a page per se, but rather an indicator of which link in the nav bar should be considered active, so NavbarIndicator is much better name. Finally, since some pages have no corresponding link in the nav bar, we now call it None (was Other).
tad-lispy
changed the title
It's a work in progress, because:
1. The items are not visible
It is not clear to me why. I will investigate after the weekend.
2. The code is somewhat spaghetti like
With 5-argument functions and such. I hope to refactor it once I got the
basic functionality.
What I did so far is:
Turn Main.Model into a record, with the union type previously called Model
renamed to Page and tracked in model.page field. There is also another field
called navbar with Bootstrap.Navbar.State type. It keeps the state of the
navbar. Helper functions had to be adjusted to this fundamental change and some
of them have pretty weird signatures ATM. Generally in my experience it's never
good idea to make Main.Model a union type. I tried it few times and it always
backfires as soon as the app needs to keep some additional piece of information
that is shared between different states. I'm surprised that Richard Feldman
modeled it that way.
The init returns the initial navbar command and the Navbar.subscriptions is
always there.
The biggest changes are in `Frame` module which is responsible for wrapping
content of each page with header and footer. The navbar is in the header and
needs to get it's config (including `Main.Msg` constructor) and state from
`Main`. So the changes propageted to `Main.view` which calls the aforementioned
and need to pass these additional parameters now. My feeling at this time is
that separating `Frame` from the `Main` module complicates things
unnecessarily. Perhaps on Monday I'll try to merge them together and see how it
will work. But I don't think the main issue (items not being visible) is
related to this.
|
The navbar takes more effort than I initially expected. Here is the update before the weekend. It's a work in progress, because:
What I did so far is: Turn The The biggest changes are in |
|
I'd like your input here @zupo . TLDR: Do we need to make our fork visually identical to the original or is it enough if it has all the functionality and looks decently? Both options are possible but I need to know where to invest the time. The issue with hidden navbar items seems to be related to the style sheet and HTML templates of the Real World SPA. With standard Bootstrap style sheet the items are displayed. I'm not sure what's going on there, as the stylesheet is complex, served minified and there is this gothinkster/conduit-bootstrap-template#5. Also the HTML generated by Elm Bootstrap is different from the one used in Elm SPA templates, which makes it tricky to figure out what exactly causes the issue. The HTML generated by Elm Bootstrap seems more correct (or at least more in line with the examples from the Bootstrap website). My opinion is to get rid of the provided CSS and use standard bootstrap (so the UX would be somewhat different) and then (if we want to invest more time) recreate the look and feel of the RealWorld SPA. |
👍 |
I made some small adjustments in the stylesheet to make the user picture look correctly in the navbar.
|
As discussed I've replaced the style sheet. The Navbar works correctly, but looks bad - colors and fonts are different and items are aligned to the left instead of to the right. Fixing the alignment seems to be tricky - see rundis/elm-bootstrap#48. Simply put, to change the alignment of the items we would have to rewrite each of them as a
So I would say it's a 👎 for Elm Bootstrap. The rest of the site is pretty mangled. Before merging I will try to fix it without matching the look and feel of the Real World SPA. |
In addition to Frame.elm it required some changes to index.html: - Making html and body elements 100% height - Making body a flex collumn See https://getbootstrap.com/docs/4.3/examples/sticky-footer/
Also break long lines and move deeply nested blocks in the view to a let block.
I rewrote the view to use Bootstrap grid and some features of Bootstrap 4, like stretched link instead of linking the whole content.
It's a quick fix to prevent pagination links from expanding the width of the page. In the original SPA they are wrapping (display: inline), but IMO it looks ugly. So instead of recreating the style I decided to add a little bit of logic and get a 5-pages span around the current page. With a little more effort it should be easy to add [ first ], [ previous ], [ next ] and [ last ] buttons, but for now I want to cover the basics and this seems good enough. There are bigger issues around pagination anyway: - It is not preserved when navigating back and forth. Navigate to page 2, then to one of the articles and back - you will get to page 1 instead of 2. - When navigating to a different page the pagination links are updated immediately, but the page content stays the same for fraction of a second before the HTTP response comes and gets decoded. It makes a weird UX where the navigation seems broken and user (me?) wants to click again. But then new content appears and click goes to some random element 😬. These two problems are not spacific to the Elm implementation. The React - Redux (most popular one) has them too.
Break long lines and deeply nested structures.
The comments section below the article is not done yet.
Also wrap the comments section in proper Bootstrap grid. The comments themselves are not done yet.
Custom CSS rule was required for comment's author image. The images are not guaranteed to be square, so using Bootstrap's fluid-img class sometimes produces distorted images.
The responsivness is not perfect - particularly on small screens there is no vertical space between buttons. I hesitate to change the definition of the buttons, as they are reused in multiple places. Also I looked at the upstream Elm implementation and React - Redux one and they are far worse in this respect so I will pause here and call it decent. We can address it in a separate PR.
Follow and favorite buttons.
Just to make it look somewhat more similar to the original app. Matching exact color should happen in a separate PR.
On Chromium, OSX the navbar was shrinking with content overflowing the height of the viewport. Interestingly on Firefox it was not happening. I'm not sure which behavior is correct, but with additional class (flexbox-shrink-0) it works as intended in both browsers.
Use flexbox and CSS to control the article info layout instead of nested Bootstrap grid.
|
I think all pages look decent on desktop monitors. On mobile screens it looks like crap, but the fix for that seems outside of the scope of this PR. It's like that in all implementations of the RealWorld and is not related to Navbar. I'll merge this, open a separate PR for that and then other PRs for various other issues. |
The nav bar is a shared component displayed on every page. Let's take it as a first challenge in our effort to use Elm Bootstrap.