minnojs / minno-quest

A js framework for creating questionnaires
http://minnojs.github.io
Apache License 2.0
4 stars 3 forks source link

The demo skin in manager 0.1 #151

Closed baranan closed 7 years ago

baranan commented 7 years ago

When I use API.addSettings('skin','demo'); in my manager file, I get the wrong buttons layout: image

When I comment out that line, I get the correct buttons: image

I am using manager 0.1. I understand that the default skin is 'demo' so I don't need to use it in the manager, but why would using it change the style? It doesn't seem like the correct behavior.

I found this with this study: https://app-prod-03.implicit.harvard.edu/implicit/showfiles.jsp?user=yba&study=relmem1 With this questionnaire: https://app-prod-03.implicit.harvard.edu/implicit/user/yba/relmem1/mem.js But, the study is complex, and I removed the comment, so I'm not sure these files will help you debug the issue.

eladzlot commented 7 years ago

It is not called the "demo" skin anymore, it is simply called "default". That made most sense to me. Can be changed if you like...

On Sat, Apr 15, 2017 at 7:42 AM, baranan notifications@github.com wrote:

Assigned #151 https://github.com/minnojs/minno-quest/issues/151 to @eladzlot https://github.com/eladzlot.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/minnojs/minno-quest/issues/151#event-1043998464, or mute the thread https://github.com/notifications/unsubscribe-auth/AByZF4_w-JfQ0qccfQSp4mwSsS8bjE0jks5rwEqkgaJpZM4M-P-y .

baranan commented 7 years ago

But why should using that code [API.addSettings('skin','demo');] create a different style? Is it an actual style that is presented as designed? That is confusing if 'demo' means one skin in version 0.0 and another skin in version 0.1. We can assume that many researchers would still have that line in their code, with the intention of showing the style that is shown by default in the current version.

eladzlot commented 7 years ago

This change is the reason that we changed versions between 0.0 and 0.1 We wanted to remove the demo skin because it doesn't make sense in a general context (as opposed to our system). It shouldn't be difficult to put it back in as a synonym to default.

On Tue, Apr 18, 2017 at 3:04 PM, baranan notifications@github.com wrote:

But why should using that code [API.addSettings('skin','demo');] create a different style? Is it an actual style that is presented as designed? That is confusing if 'demo' means one skin in version 0.0 and another skin in version 0.1. We can assume that many researchers would still have that line in their code, with the intention of showing the style that is shown by default in the current version.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/minnojs/minno-quest/issues/151#issuecomment-294809382, or mute the thread https://github.com/notifications/unsubscribe-auth/AByZF5MkL39JT4p5OtX6R6WNZZvbbvvoks5rxKbfgaJpZM4M-P-y .

baranan commented 7 years ago
  1. The serious problem is that is has an effect on the style, but not the expected effect.
  2. It should simply do nothing or be a synonym of default (because it is the default).
andrewdzik commented 7 years ago

So currently if you do something like API.addSettings('skin','test123'); then it will have the same behavior? What do we want to happen when you try to load a skin that does not exist?

On Tue, Apr 18, 2017 at 8:34 AM, baranan notifications@github.com wrote:

  1. The serious problem is that is has an effect on the style, but not the expected effect.
  2. It should simply do nothing or be a synonym of default (because it is the default).

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/minnojs/minno-quest/issues/151#issuecomment-294822040, or mute the thread https://github.com/notifications/unsubscribe-auth/AED7_BZwINDpib9Fdg98dnMsyi1VioNPks5rxK3zgaJpZM4M-P-y .

baranan commented 7 years ago

The skin should not change from the default skin if someone requests a skin that does not exist. If I understand correctly, that was the bug I reported at the top of this thread.

As for how to notify to the user that the skin does not exist, I think you can use a comment in the console that is also shown at bottom of the screen in the development server (as you do with other messages, like when a task is refreshed and you notify that a task has been saved twice).

eladzlot commented 7 years ago

For now, I simply added a demo skin that is identical to default. It's the simplest way to avoid this problem...