#333 Ban List improvments


Changed editor layout (now looks much better), look at example screenshot for details.

(rewrite src/mumble/BanEditor.ui (79%))


  • Benjamin Jemlich

    1) Reread the Qt docs on translation strings
    2) Fix the indentation (Mumble uses tabs, not spaces)
    3) Don't remove the Username field
    4) Mixing different data (IPs/Usernames) in the same list is unwise
    5) Don't center text in the search field if it's != the default text

  • Zuko

    Zuko - 2011-02-21

    1) later
    2) no difference for me (qt creator makes that)
    3) Username field is no longer needed
    4) It's not my fault, its' not possible to set nickname for "manual" bans
    5) what!?

  • Thorvald Natvig

    Thorvald Natvig - 2011-02-21

    What pcgod said about tr().

    I'm also not terribly happy about mixing usernames and IPs in a single list. What if you want to search by a specific IP? There's no way to show them unless you click every single ban. It's probably better to add a column for the usernames.

    You need to modify the alignment of the input field when users start inputing. Have a look at the chatbar for an example.

    Why do you move
    b.qsUsername = u8(be.name());
    up two lines?

    BTW, for QStrings, don't compare length() with 0, use .isEmpty().
    Also, in your modified Ban lessthan operator, you need to compare the equality of the tolower'd strings, since that is what you use to sort by. Have a look at http://doc.qt.nokia.com/latest/qstring.html#compare-6 for a more efficient solution :)

  • Zuko

    Zuko - 2011-02-21

    thanks for comment its really helpful
    I could add a column with IP, but when you ban someone you see his nickname and not the IP (so you remember that "pcgod" is banned and not eg.

    "Why do you move
    b.qsUsername = u8(be.name());
    up two lines?"
    - dunno ;D (This is totally unnecessary)

    "BTW, for QStrings"... - ok

  • Stefan H.

    Stefan H. - 2011-02-21

    Comparing user facing strings with compare (or the equivalent < overloard) is discouraged by the Qt documentation. Instead use localAwareCompare which also removes the need to match the cases of the string because it doesn't do a simple minded unicode code comparision as compare does.

    See: http://doc.qt.nokia.com/latest/qstring.html#querying-string-data

  • Zuko

    Zuko - 2011-02-22

    Example screenshot

  • Zuko

    Zuko - 2011-02-22

    "new version"

  • Thorvald Natvig

    Thorvald Natvig - 2011-02-25

    Ok :) I did some testing on this, and a few issues have shown up.

    When you start to write in the 'Who are you looking for?' field, the input text is still centered and italics. It needs to revert to default-justified plaintext when you are editing it.

    All of the widgets seem to be missing tool tips and whatsthis; these need to be added.

    It's a bit unclear how to add new bans from scratch. If you have an existing ban selected and start editing, you can only update.

    If I hit Clear, Add becomes disabled. If I then fill out the name, nothing happens. If I fill out Reason, Add becomes enabled. But hitting Add does nothing at this point. Or, actually, it updates the Start date. This logic needs to be fixed.
    While we're at it, hitting Clear, then modifying the end date enables Update. Update of what? :)

    For slots (and elsewhere it makes sense), please use const QString & instead of passing QString.

    Please name your QGroupBox, it is necesarry for skins.

    on_qpbClear_clicked() first clears all the fields, then checks if they are cleared. They ARE cleared.

    Rather than have the 'enable/disable' fields in BanEditor::on_qleSearch_textChanged, it is probably better to have this in on_qlwBans_currentRowChanged.

    For added bonus, make the 'is modified' for enabling 'Update' compare against the stored ban.

    We're moving forward :)

  • Kissaki

    Kissaki - 2013-03-22
    • status: open --> wont-fix
    • milestone: --> General

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:

No, thanks