New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make mu (even more) friendly for dyslexic users #633

Open
tim-mccurrach opened this Issue Aug 30, 2018 · 12 comments

Comments

Projects
None yet
3 participants
@tim-mccurrach
Copy link
Contributor

tim-mccurrach commented Aug 30, 2018

Mu actually does a really great job of this already, there are loads of aspects of the design that make it good for people with dyslexia. I had a few thoughts about what else could be done though.

  • Lots of people who suffer ‘visual stress’ will use coloured overlays to help with reading performance, and the ability to customise the background/font colour can be helpful when reading off a screen. However the specific colour of the overlay can be important, and different people will need different colours. This will in turn mean you would also need to be able to customise the colours used for syntax highlighting, since there would probably be inevitable clashes. There are obviously, quite a few considerations and questions to ask in terms of how this could be best done, whilst maintaining the clean simplicity of mu’s design, but think it is definitely worth thinking about.

  • The main principle above is that a high contrast can be difficult to read. In particular pure white, with pure black (as is used in the Day theme) can cause visual stress, and a slightly off white, with a very dark grey text is much easier to read. Such a change would make the default position much easier for dyslexic users, and make either no (or a slightly positive) difference to non-dyslexic users. Is it okay for me to just make a pull request here?

  • I was also wondering about providing the option of a dyslexic-friendly font. There are quite a few out there. The current font is good as it is sans-serif which is generally helpful, but there are some fonts designed specifically for people with dyslexia that might help a lot. Perhaps this could be an option in the mu-admin dialogue?

  • One interesting thing that I have found is (and this might just be me) I find the toolbar noticeably more difficult to navigate in the night and contrast modes, because the separators are virtually invisible. I’ve had some difficulty adjusting these settings, the QToolbar::separator selector allows some customisation, but I can’t find a property that adjusts the colour of the thin line in the middle. Is this possible?

@ZanderBrown

This comment has been minimized.

Copy link
Contributor

ZanderBrown commented Aug 30, 2018

  1. Like the idea but I'm not sure how doable it is, can see the benefits but adds a lot more settings/moving parts and the CSS is getting silly complex already. In an ideal world I'd rewrite it in less but I don't want to introduce extra (& non-python) build deps. Personally I'm red-green colour deficient which might explain some of the current colour choices :-)

  2. Sure! Seems reasonable, like you say it won't be noticeable for most but may help some

  3. If you can find a suitably licensed monospace font that sounds good, might cause more problems for Debian though...

  4. That's semi-deliberate but QSS (Qt-CSS) does give us quite a bit of flexibility on this, what sort of thing where you looking for? I'm reluctant to change this too much now we have released 1.0

@tim-mccurrach

This comment has been minimized.

Copy link
Contributor Author

tim-mccurrach commented Aug 31, 2018

  1. I definitely agree – it could potentially be a real mess if not done carefully, and certainly needs some thinking about. I guess I mainly wanted to just float the idea, as it could make a real difference. (Could potentially be useful for people with other forms of colour-blindness too, although I'm afraid I am slightly ignorant in this area so don't know).

  2. Great, I’ll get on with this soon 😊

  3. Having now done a bit of googling, I can’t really find anything yet that is both suitable and that I’m also convinced would actually be beneficial , but I’ll keep an eye out.

  4. It’s just that the separator which is visible with the day theme, becomes virtually indiscernible in night mode rendering it a bit pointless (pun not intended lol) – compare fig 1 and fig 2. I couldn’t find a property that seemed to adjust the line in the middle. Trying various things (color, background-color, foreground-color etc) the best I could get was what’s in fig. 3. I was asking if there was a property which would adjust the colour of just the line down the middle, not the whole thing. Since then I did think of the following:

QToolBar::separator {
    background: #6b6b6b;
    width:1px;
    margin:2px;
 }

Which achieves the desired result (see fig 4). I just chose the same colour as is used for the borders below. It feels like a bit of a hack, but I don’t think such a bad one as to be completely avoided. What do you think?

image

Sorry btw if this seems really really pernickety, I've just noticed that in day theme my cursor seems to know where to go, since there are clear sections, but in other themes I seem to have to search along the whole block to find the desired button.

ZanderBrown added a commit to ZanderBrown/mu that referenced this issue Sep 11, 2018

Toolbar styles for mu-editor#633
An 'opps i thought i commited & pushed this a week ago' commit
@tim-mccurrach

This comment has been minimized.

Copy link
Contributor Author

tim-mccurrach commented Sep 11, 2018

So having done a bit of reading on the issue of colours and readability, all the literature is in agreement that pure white, and pure black is the worst combination for reading, so I have made a pull request to adjust that. The general consensus though, is that to significantly help most people suffering with visual stress/Irlens Syndrome they need to select specific colours.

I agree the styling would need some refactoring before we tried to do something like that. As I see it, the main issues are (I hope this is not read in the wrong way, I say this with the absolute greatest respect, and have learned lots from just reading the mu code-base) :

  • The styling is split between the Theme subclasses, the stylesheets in resources, and lots of snippets of code in lots of different places (often with if a… if b…)
  • The functions set_theme sometimes accept strings as arguments, but other times accept 'X-Theme objects' – this is a little confusing scanning through the code.
  • There is quite a lot of repeated QSS in the three stylesheet files.

I propose as an alternative, we store all theme-specific style information in the classes defined in themes.py:

  • To deal with the stylesheets, there could be one stylesheet with placeholder values, which would then be replaced by values stored in each Theme sub-class.
  • Add a next_theme() method to Theme which returns the next theme to be used to simplify the logic of switching themes.
  • Always pass around the actual 'X-Theme objects', rather than strings to set_theme fuctions.

Most situations could then be dealt with by adding a property to of the each classes inheriting Theme.

For example instead of (in panes.PlotterPane)

def set_theme(self, theme):
    if theme == 'day':
        self.chart.setTheme(QChart.ChartThemeLight)
    elif theme == 'night':
        self.chart.setTheme(QChart.ChartThemeDark)
    else:
        self.chart.setTheme(QChart.ChartThemeHighContrast)

We could simply have:

def set_theme(self, theme):
    self.chart.setTheme(theme.ChartTheme)

This would mean all the theme information was in one place, would simplify the code everywhere else, and wouldn’t introduce any new dependencies.
Likewise (in main.window):

def set_theme(self, theme):
    self.theme = theme
    self.load_theme.emit(theme)
    if theme == 'contrast':
        new_theme = ContrastTheme
        new_icon = 'theme_day'
    elif theme == 'night':
        new_theme = NightTheme
        new_icon = 'theme_contrast'
    else:
        new_theme = DayTheme
        new_icon = 'theme'
    for widget in self.widgets:
        widget.set_theme(new_theme)
    self.button_bar.slots['theme'].setIcon(load_icon(new_icon))
    if hasattr(self, 'repl') and self.repl:
        self.repl_pane.set_theme(theme)
    if hasattr(self, 'plotter') and self.plotter:
        self.plotter_pane.set_theme(theme)

would become

def set_theme(self, theme):
    self.theme = theme
    self.load_theme.emit(theme)
    for widget in self.widgets:
        widget.set_theme(theme)
    self.button_bar.slots['theme'].setIcon(load_icon(theme.icon))
    if hasattr(self, 'repl') and self.repl:
        self.repl_pane.set_theme(theme)
    if hasattr(self, 'plotter') and self.plotter:
        self.plotter_pane.set_theme(theme)

As well as being simpler, and more maintainable. This would also pave the way to allow custom settings for users who wanted them. We could simply have a fourth class CustomTheme. Since the logic involved in switching themes would be in next_theme the rest of the code wouldn't even need to be altered.

(This would also separate the logic of switching themes and the actual colors used from the rest of the UI Layer, and would make things easier for porting to different UI library.)

A few things such as apply_to in Theme would have to work slightly differently. I know this is just a sketch, so perhaps it’s difficult to answer the following questions, but: is there anything I have overlooked? Or potential disadvantages to this way of doing things? It will take a little work, but it definitely doesn’t seem overly arduous.

@ZanderBrown

This comment has been minimized.

Copy link
Contributor

ZanderBrown commented Sep 11, 2018

Sounds great, some notes:

  • contrast is quite different from day/night
  • day uses a different tab colour to night/contrast

But in theory regexing a base theme would make a lot of things easier

I would prefer it if set_theme went away with #530 and they (panels/editors) instead connected to a signal but passing a Theme instance would make a lot of sense (esp for the level of customisation needed)

Qt has a colour picker so that helps (note to self: dlg.selectedColor().name()) and most colours are already set relative to each other which could be done with QColor by the looks of things

Persistence wise we could store the colours in a colour dict in session.json

</brain-dump>

ntoll added a commit that referenced this issue Sep 29, 2018

@ZanderBrown

This comment has been minimized.

Copy link
Contributor

ZanderBrown commented Oct 2, 2018

I have a WIP branch for this now, pending disaster I hope to have it ready for Mu-moot

@ntoll

This comment has been minimized.

Copy link
Member

ntoll commented Oct 2, 2018

👍 :-)

@tim-mccurrach

This comment has been minimized.

Copy link
Contributor Author

tim-mccurrach commented Oct 2, 2018

I’ve also had a WIP branch which has sat dormant for a while. I’ve uploaded it, and the link is here. At the moment what I’ve done is refactored so that everywhere there is a self.theme, or where theme is passed around, it refers to a Theme instance. This has removed all of the logic of changing theme to a method next, so it is easy to add a CustomTheme. Hopefully it’s simplified a lot of the logic elsewhere.

Since theme is assigned during start-up, logic.py needs to import Theme or DayTheme and importing anything from interface involves importing window which then gets us into trouble at this point. I therefore moved themes.py into resources.

Feel free to use/chage/ignore any of what I’ve done. I’d love to collaborate on this if possible, although I’m not sure how that is best done.

(Sorry, that the branch also seems to have some commits from ages ago, which I made from my master branch, I’ll fix this at some point soon. It’s just the final commit that is relevant.)

Only other thoughts are:

  • to implement the QSS stylesheets could we use a templating engine such as Jinja2?
  • the JupyterREPL might be trouble. It writes coloured text using a combination of ANSI codes, HTML stylesheets, and a QSyntaxHighlighter. Consequently when you change theme, lots of the coloured text doesn't update. At the moment I'm looking at how to re-impliment the QSyntaxHighlighter to solve this.
@ntoll

This comment has been minimized.

Copy link
Member

ntoll commented Oct 3, 2018

Folks, it is great to see this work moving forward!

A quick note: Mu's code base is relatively simple, and I'd like to keep it that way. Please try to do the simplest thing first (i.e. obvious names, obvious code in the obvious place). If there's a problem widget (e.g. the JupyterREPL) then it's far better for upstream to fix the problem with out help (I know the maintainer of JupyterREPL), than for us to create a "hacky" sticking plaster because everybody gets the benefit of an upstream change.

Hope this makes sense.

@tim-mccurrach

This comment has been minimized.

Copy link
Contributor Author

tim-mccurrach commented Oct 4, 2018

I can definitely appreciate that, it'll make solving the problem cleaner and simpler too :). I'll raise an issue at the qtconsole repo.

@ZanderBrown

This comment has been minimized.

Copy link
Contributor

ZanderBrown commented Oct 4, 2018

Hmm sure I posted a response before

My solution is quite similar in passing Theme objects around instead of names and uses some regexing upon a standardised base theme to reimplement day/night

@tim-mccurrach

This comment has been minimized.

Copy link
Contributor Author

tim-mccurrach commented Oct 4, 2018

sounds good. I didn't get as far as re-implementing the day/night theme's so happy to go with your solution :)

@ntoll

This comment has been minimized.

Copy link
Member

ntoll commented Oct 4, 2018

I'm really looking forward to reviewing this. :-) This is great stuff. Thank you.

murilopolese added a commit to murilopolese/mu that referenced this issue Oct 9, 2018

Toolbar styles for mu-editor#633
An 'opps i thought i commited & pushed this a week ago' commit
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment