In the numerical systems or "programmer" mode, a click is required to see the current number in another base. Having the conversions visible immediately as input changes is more convenient.
Details
Details
- Reviewers
cfeck dakon - Group Reviewers
VDG - Commits
- R353:1015fa9ac9a4: Add live conversions between numerical bases
Diff Detail
Diff Detail
- Repository
- R353 KCalc
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Comment Actions
First I'm looking for UI feedback. My current attempt looks like this:
I'm far from a usability expert or being a GUI programmer. This layout seems quite natural to me, but another option would be to place the conversion radios and labels at the far left (that is, left of the button pad), which I briefly tried but didn't like it. Somehow doesn't feel right not to have a keypad start at the edge.
Also I used KSqueezedTextLabel for the conversions - is that an appropriate choice?
(Side note: KCalc code seems a bit old, and does stuff inconsistent with Qt guidelines, such as having tabs for indents but with some spaces mixed in - I've intentionally formatted my code keeping the surrounding style)
Comment Actions
I personally like it and it would often be helpful for my personal use, so I guess if noone complains just go for it.
| kcalc.cpp | ||
|---|---|---|
| 1882 | converted is not needed, just put the QStringLiteral in the next line where it is used. | |
| 1982 | and this becomes bad if this stays a QList, because it detaches (i.e. copies) the container for the iteration, it needs at least qAsConst(). No problem if it is a std::array as those do not detach, but the Qt types need this. | |
| 2006 | same here | |
| kcalc.h | ||
| 270 | even if the code above also uses it, QList is the most awful container one can use. Either use QVector or just use std::array, this has a fixed size and needs no dynamic resizing. A followup patch to convert the QLists above would be welcome ;) | |
Comment Actions
I'm glad it's not just me who has this as a major use case for calculator applications.
Comment Actions
@dakon would you agree with this revision? Doesn't seem like anyone else has feedback.
| kcalc.cpp | ||
|---|---|---|
| 103 | Heh, on the contrary, this one is with spaces while the rest of kcalc is in tabs with the old style. Fixing... | |
| kcalc.cpp | ||
|---|---|---|
| 1882 | You convert the KNumber to a string in line 1879, and then convert it back to an uint64 here to be able to convert it back to a string. This does not make any sense. Put it in an quint64 and use that in all following steps. | |
Comment Actions
So much for touching unfamiliar QT types while not properly concentrating, ended up with some absurd code :)
Comment Actions
Looks good to me so far. The size in the ui file is now much bitter than before, please check if that is really needed. Otherwise feel free to push.
Comment Actions
The change in the ui file is pretty big because I removed the base controls from their previous layout and put them in a new one, which caused Qt Designer to output quite many changes as the number of layouts has changed. Seems to me like it's needed.
I have no push permissions, so I'll need someone's help to land this.
Comment Actions
please do a git commit and then "git format-patch -1" and send me the resulting file via mail, then I'll apply this.