Skip to content

Commit

Permalink
Add space between navigation links
Browse files Browse the repository at this point in the history 
Makes the controller interface friendlier for text-only browsers/any use
without stylesheets.
  • Loading branch information
@knuton
knuton committed Jul 5, 2023
1 parent 1a11ad4 commit abc957b
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion controller/server/view/common/page.ml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ let html ?current_page ?header content =
~a:[ a_class [ "d-Layout__Aside" ] ]
[ nav
([ Info; Network; Localization; SystemStatus; Changelog ]
|> List.map (menu_item current_page))
|> List.concat_map (fun page -> [ menu_item current_page page; txt " " ]))

This comment has been minimized.

Sign in to view

Copy link
@guyonvarch

guyonvarch Jul 5, 2023

Member

Add a small comment to explain the reason to add text spaces ?

This comment has been minimized.

Sign in to view

Copy link
@knuton

knuton Jul 5, 2023 •
edited
Loading

Author Member

Seems a bit too much to me, because then we could add a lot of comments. 😄

I mean, we are just producing standard HTML here and we want spaces between a series of inline elements, which is nothing too crazy. It just looks a bit crazy in tyxml, but it's the most normal thing in the world.

Maybe add a general mention that having the UI be basically usable as an unstyled web page is an ambition? (e.g. here)

This comment has been minimized.

Sign in to view

Copy link
@guyonvarch

guyonvarch Jul 5, 2023

Member

It’s not a question of standard HTML, but if I update this code in the future, I may forget about the reason we added space, and remove it along the way.

This comment has been minimized.

Sign in to view

Copy link
@knuton

knuton Jul 5, 2023

Author Member

I assumed as much, I am just thinking that there are a lot of other places that at the moment "accidentally" work in lynx or elinks (and some things still don't work well right now), but that we can mess up very easily by accident.

And I don't want to litter the code with specific comments when the overall goal is just "use markup that renders reasonably when unstyled".

And I don't want to complicate the controller with this at all, the motivation to use lynx was only that it is easy to set up on a virtual console and does the job sufficiently for now.

If it causes complication, I would rather drop this change and spend the time on setting up a GUI virtual console at some point instead.

This comment has been minimized.

Sign in to view

Copy link
@guyonvarch

guyonvarch Jul 6, 2023

Member

I don’t think it complicates much the code, but let’s move forward.
; div
~a: [ a_class [ "d-Layout__Shutdown" ] ]
[ menu_item current_page Shutdown ]
Expand Down

0 comments on commit abc957b

Please sign in to comment.