mu-editor / mu

A small, simple editor for beginner Python programmers. Written in Python and Qt5.
http://codewith.mu
GNU General Public License v3.0
1.41k stars 435 forks source link

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

Open tim-mccurrach opened 6 years ago

tim-mccurrach commented 6 years ago

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.

ZanderBrown commented 6 years ago

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 commented 6 years ago
  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.

tim-mccurrach commented 6 years ago

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) :

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

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 commented 6 years ago

Sounds great, some notes:

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

\

ZanderBrown commented 6 years ago

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

ntoll commented 6 years ago

:+1: :-)

tim-mccurrach commented 6 years ago

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:

ntoll commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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

ntoll commented 6 years ago

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