More clock options #3207
More clock options #3207
Conversation
|
When I submitted this I was thinking only Tweak the styling perhaps so we can put all three view options on one line? |
sulkaharo
left a comment
sulkaharo
left a comment
There was a problem hiding this comment.
Sorry for slow review! Can you also include the change to the language.js that includes the corresponding localization keys?
|
Updated the nav to show all three clocks in one row. Looks like this: Conflict with the dev progress on language.js there should be easily resolved; stems from the change to the plural "Clock Views". |
|
Any chance someone could offer a hand in finalizing this one? I don't think I'm able to resolve the language.js conflicts, or at least I'm not sure how to do that on my end. Should be an easy fix, and then this one could be merged. |
|
@unsoluble I resolved the merge errors for you. Is this ready to merge? LGTM but I didn't test it yet. |
|
Thanks! I think it should be good to go, yep. |
|
Ok, I'll test it myself later this week and have a look at why this PR adds language lines. Then I merge it, unless @sulkaharo or @MilosKozak beat me. |
|
@unsoluble according to @sulkaharo there are some issues with this PR with mobile devices, see I haven't tested this PR on mobile devices myself. |
|
Could try to fix if I had more information — not clear what the concern is. |
|
This is what it looks like on an iPhone SE, which has one of the smallest screens on the market right now: |
views/index.html
Outdated
| <li><a id="admintoolslink" href="admin" target="admintools" class="translate">Admin Tools</a></li> | ||
| <li><a id="clocklink" href="bgclock.html" target="admintools" class="translate">Clock View</a></li> | ||
| <li class="multilink"> | ||
| <a>Clock Views:</a> |
There was a problem hiding this comment.
The Clock Views part is not translatable
|
Small issue: |
|
I don’t know what you mean by translatable. Can you elaborate? |
|
@unsoluble I added translation strings for it, but it does not translate. See
I expected: Klokweergave instead of Clock Views |
|
Oh I see it: should be a class=translate in that A tag. Can you add that? Would myself but will be on my phone only for a while. |
|
@unsoluble : please use https://github.com/PieterGit/cgm-remote-monitor/blob/201807_clock/lib/language.js to resolve the merge conflict. I think this PR is ok and can be merged after the merge conflict has been fixed. @sulkaharo can you confirm there are no problems with this PR anymore? |
|
This conflict-resolving stuff is new to me; I'm not sure if I'm approaching it the right way. Tried to just commit with the updated contents of language.js that you linked, but that doesn't seem to have done the trick. Is there something else I need to do on my end? |
More clock options #3207 and some dutch language updates
|
Merged with #3712 |
4 participants
Should have links to both bgclock.html and clock-color.html in the main link panel. Might be a tidier way of putting them both on one line, but didn't want to mess with the CSS.