Skip to content
This repository has been archived by the owner on Apr 8, 2020. It is now read-only.

Feature ngm theme #309

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

pfitzpaddy
Copy link

Hi Martijn, I have a new theme for review, essentially just including Roboto font type and other minor cosmetic overrides within 'theme-ngm/_styles.scss'.

If there is a more convential way to include a style (i.e. enketo-core) or any changes you would like to see let me know. Attached is a screenshot of the form.
theme-ngm

@MartijnR
Copy link
Member

MartijnR commented Sep 9, 2015

Thanks. I will have a look! Screenshot looks very interesting. The theme should live in enketo-core ideally, but I will have a look here first.

First the most important thing. What does ngm stand for?

@pfitzpaddy
Copy link
Author

Thanks Martijn, I would appreciate any tips since I have never worked with sass before and also, happy to place these changes in enketo-core if it gets the nod.

ngm stands for Angular-Materialize, i have built a widget dashboard framework that implements the angular-dashboard-framework with Materializecss. The framework is complete although the demo page and supporting widgets that will help others use it are still in progress which i hope to have on github soon.

@MartijnR
Copy link
Member

MartijnR commented Sep 9, 2015

This theme looks very good! A very nice addition and very different from existing themes (I see now that it extends the Grid theme). I'll be adding my comments below as I go through and will add sass comments inline (but it may take a while).

@MartijnR
Copy link
Member

MartijnR commented Sep 9, 2015

  • we need an easy one-word theme name. immap? chic? fitzgerald? ;), unhcr?, something easy to remember because users will be able to enter this name in their XLSForm definition to pick this theme
  • .DS_Store files not to be committed
  • radiobuttons and checkboxes look the same (probably icon font is not loading correctly)
  • print button not shown (probably same issue as previous)
  • not so crazy about the selected vs unselected radio/checkbox. Too subtle. (probably same issue as previous). This is what I see:
    screen shot 2015-09-09 at 4 53 17 pm
  • the hover effect on invalid questions is cool. There is an issue in the widgets form with the select multiple question. When you select c and d together you won't get the immediate obvious feedback until you focus on another question (just the error message). Not sure whether that is desirable.
  • the radiobuttons/checkboxes on touchscreens (simulate with ?touch=true) are the same, not so suitable for touchscreens imo
  • timepicker chevron buttons not displayed
  • am a little concerned about the number of font files to be cached in ApplicationCache manifest, but that's to be sorted out somehow (by me). I like this Roboto font a lot. Will check browser support and whether there is still a need for all 4 font formats in modern browsers.
  • the focus bottom-border is not working on multiline text inputs (textarea element)
  • repeat is not visually different from non-repeat (this is nice dev form with all widgets inside a repeat)
  • repeat buttons invisible (this may be the same icon font issue)

Still going to look at nested repeats, right-to-left form language, code, grid functionality

@pfitzpaddy
Copy link
Author

Thanks Martijn.

There are plenty of issues, the project im working on is still in its early stages so my timeframe is a couple of months. I will aim to have a new font with all points you mention before then. Thanks for your help.

ToDo;

  • New theme name
  • Investigate fonts not loading (the sample I have worked with looks ok, sounds like an issue is in there somewhere tho)

checkboxes

  • Add bold to selected option label
  • Investigate hover effect
  • Review touchscreen radiobuttons/checkboxes
  • Investigate timepicker chevron buttons
  • Reduce Robot font formats (especially since only basic is in use)
  • "textarea" element issue
  • Repeat is not visually different from non-repeat issue

@MartijnR
Copy link
Member

Thanks a lot. I'll still look at things I didn't have time for yesterday, when I have time in the next week. I realized belatedly, that this theme extends the grid theme and supports columns. In that case it would be good to have 'grid' be a part of the name (to make sure the grid-specific print script is loaded).

@MartijnR
Copy link
Member

Also thanks for indirectly introducing me to IT Crowd after @dorey of the KoBo team figured out your avatar. Am having fun watching this show. 😆

@pfitzpaddy
Copy link
Author

If I do nothing else this year then I can claim that. Enjoy, it's one of my favorites!

@richardokonicha
Copy link

hey! where are you guys?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants