Recorder adjustments #2427

pljones · 2022-02-23T17:16:04Z

Short description of changes

Tiny, tiny UI improvement, plus code changes to bring into line with UI. Plus Qt6 bits for recorder.

Before (Windows)

Server Setup tab:

What used to happen was the status text would change length making the session box change size. Also, the session box is used for error messages.

Options tab (running):

Log:

Recording state: enabled
2022-02-23 16:15:03, 192.168.1.5, connected (1)
2022-02-23 16:54:55,, server idling -------------------------------------
Session RPP: "D:/TEMP/recording/Jam-20220223-161503768/Jam-20220223-161503768.rpp"
Session LOF: "D:/TEMP/recording/Jam-20220223-161503768/Jam-20220223-161503768.lof"

After (Linux)

Server Setup tab:

Now the status text is the last item, so the UI doesn't "move around".

Options tab (running):

Log:

Recording state: enabled
2022-02-23 17:05:52, 192.168.1.5, connected (1)
2022-02-23 17:07:16,, server idling -------------------------------------
Session RPP: "/tmp/Jam-20220223-170552454/Jam-20220223-170552454.rpp"
Session LOF: "/tmp/Jam-20220223-170552454/Jam-20220223-170552454.lof"

Context: Fixes an issue?
UI improvement, Qt6 compliance.

Does this change need documentation? What needs to be documented and how?
Server screenshots.

Status of this Pull Request
Complete.

What is missing until this pull request can be merged?
2x Approves.

Checklist

I've verified that this Pull Request follows the general code principles
I tested my code and it does what I want
My code follows the style guide
I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
I've filled all the content above

pljones · 2022-02-23T17:31:38Z

src/serverdlg.cpp

        "<br/><b>" + tr ( "NOTE" ) + ":</b> " +
-        tr ( "If the recording directory is not useable, the problem will be displayed in place of the directory." ) );
+        tr ( "If the recording directory is not useable, the problem will be displayed in place of the session directory." ) );


Translations will definitely change...

pljones · 2022-02-23T17:32:58Z

src/serverdlg.cpp

        "</dd>" + "<dt>" + SREC_NOT_ENABLED + "</dt>" + "<dd>"
 #ifdef _WIN32
-        + tr ( "Recording has been switched off by the UI checkbox" )
+        + tr ( "Recording has been switched off by the UI checkbox." )


Tiny translations change.

With small, non-language changes like this, it is probably a good idea manually to update the .ts files to match. Otherwise lupdate creates it as a new, untranslated entry, and the person translating needs to notice that there is a similar vanished translation that could be used.

That's a bit scary as I never look at them... I'll have a go...

@softins I've added a new commit with my attempt in, including the updated ts and generated qm files.

Should I have tagged the translation entry at all?

Separate topic: I did notice a lot of translation type="vanished" entries - are those what would have happened and do they need clearing up?

Should I have tagged the translation entry at all?

I think when we do punctuation updates and want to avoid extra work by translators by modifying .ts files directly (which I think is good), we should do the punctuation update in the translated string as well (I currently see Die Aufnahme wurde über die Checkbox in der Benutzeroberfläche ausgeschaltet without a trailing dot).

Separate topic: I did notice a lot of translation type="vanished" entries - are those what would have happened and do they need clearing up?

Yes, I think that's what would have happened and I think they should be kept and not be cleared as they may help translators in the future if this or a similar translation gets re-introduced.

OK, all entries except Chinese updated to add the ".". I don't know - and couldn't tell from context - whether or not to add it there.

(Also convinced vim that some .ts files aren't typescript but are xml.)

pljones · 2022-02-23T17:34:14Z

src/serverdlg.cpp

+    QString strCurrentSessionDirAN = tr ( "Current session directory text box (read-only)" );
+    lblCurrentSessionDir->setAccessibleName ( strCurrentSessionDirAN );
+    edtCurrentSessionDir->setAccessibleName ( strCurrentSessionDirAN );
+


Moved from line 124.

pljones · 2022-02-23T17:35:10Z

src/serverdlg.cpp

-    edtCurrentSessionDir->setWhatsThis ( "<b>" + tr ( "Current Session Directory" ) + ":</b> " +
-                                         tr ( "Enabled during recording and holds the current recording session directory. "
-                                              "Disabled after recording or when the recorder is not enabled." ) );
+    // new recording


Moved from line 146.

pljones · 2022-02-23T17:38:11Z

I've raised this because the current UI feels too cramped.

I'd also considered moving "New recording" to where the "Session" label is but I wasn't happy with that. It left the box "unlabelled" or took up too much space. It's still not ideal, so I'm open to suggestions.

ann0see · 2022-02-26T22:35:55Z

The main thing which confuses me is that the recorder is grayed out if no recording dir is set and there's no hint saying that this needs to be set in "Options". All in all, I think the GUI is still a bit cluttered. Maybe we could structure the tabs a bit like the settings (Server Profile/Recorder/Advanced settings).

Basically this would mean:

Directory, Server info, Language would go to Server Profile (maybe also the welcome message, but that could also be on the advanced tab)
Recorder (everything related to the recorder)
Advanced settings everything else.

pljones · 2022-02-27T09:56:17Z

The main thing which confuses me is that the recorder is grayed out if no recording dir is set

That's not changed, AFAIK.

I think the GUI is still a bit cluttered.

That's a separate issue. But yes - all the registration bits (Genre, Custom Directory, Location ("My Server Info")) could be one tab, the jam recorder on another and the language, delay panning, welcome message and start minimized on a third. The registration bits could then be the third tab ("out of the way").

softins

The main thing which confuses me is that the recorder is grayed out if no recording dir is set

That's not changed, AFAIK.

I see if the recording dir is not set, the status message says "Not initialised" (as opposed to "Not recording"). Maybe that message could be more explicit such as "Recording dir not set"?

Apart from that, all looks ok, and I have done a successful build and run.

hoffie

Looks good to me, thanks!

I see if the recording dir is not set, the status message says "Not initialised" (as opposed to "Not recording"). Maybe that message could be more explicit such as "Recording dir not set"?

I second that.

pljones · 2022-02-28T17:57:12Z

I see if the recording dir is not set, the status message says "Not initialised" (as opposed to "Not recording"). Maybe that message could be more explicit such as "Recording dir not set"?

Again, not a change in this PR.

There may also be more to it than just changing some text.

ann0see · 2022-07-25T17:14:58Z

Recorder adjustments is a bit vague for the changelog. Maybe add something about the fact that the GUI changed a bit.

pljones · 2022-07-26T06:13:04Z

Yeah - I couldn't remember when reviewing the ChangeLog what I'd done. This does need amending.

CHANGELOG: Server GUI: Re-order Recorder widgets for better use of space (plus Qt6 compliance)

pljones changed the title ~~Patch/recorder adjustments~~ Recorder adjustments Feb 23, 2022

pljones commented Feb 23, 2022

View reviewed changes

pljones added this to the Release 3.9.0 milestone Feb 23, 2022

pljones force-pushed the patch/recorder-adjustments branch 2 times, most recently from b81b258 to 28b9175 Compare February 25, 2022 23:17

Recorder UI adjustments

4f2b605

pljones force-pushed the patch/recorder-adjustments branch from 28b9175 to dd15853 Compare February 27, 2022 12:44

pljones added 2 commits February 27, 2022 13:07

Make minor translation changes

b83df25

Avoid QString::null (for Qt6)

5570ccb

pljones force-pushed the patch/recorder-adjustments branch from dd15853 to 5570ccb Compare February 27, 2022 13:08

softins approved these changes Feb 28, 2022

View reviewed changes

softins mentioned this pull request Feb 28, 2022

Server UI changes #2199

Merged

5 tasks

hoffie approved these changes Feb 28, 2022

View reviewed changes

softins mentioned this pull request Feb 28, 2022

Server GUI directory should default to "None" (not registered) #2441

Closed

pljones merged commit 158a0ab into jamulussoftware:master Feb 28, 2022

pljones deleted the patch/recorder-adjustments branch February 28, 2022 17:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Recorder adjustments #2427

Recorder adjustments #2427

pljones commented Feb 23, 2022 •

edited by ann0see

Loading

pljones Feb 23, 2022 •

edited

Loading

pljones Feb 23, 2022

softins Feb 24, 2022

pljones Feb 24, 2022

pljones Feb 25, 2022

pljones Feb 25, 2022 •

edited

Loading

hoffie Feb 26, 2022

pljones Feb 27, 2022 •

edited

Loading

pljones Feb 23, 2022

pljones Feb 23, 2022

pljones commented Feb 23, 2022 •

edited

Loading

ann0see commented Feb 26, 2022 •

edited

Loading

pljones commented Feb 27, 2022 •

edited

Loading

softins left a comment

hoffie left a comment

pljones commented Feb 28, 2022 •

edited

Loading

ann0see commented Jul 25, 2022

pljones commented Jul 26, 2022 •

edited

Loading

Recorder adjustments #2427

Recorder adjustments #2427

Conversation

pljones commented Feb 23, 2022 • edited by ann0see Loading

Checklist

pljones Feb 23, 2022 • edited Loading

Choose a reason for hiding this comment

pljones Feb 23, 2022

Choose a reason for hiding this comment

softins Feb 24, 2022

Choose a reason for hiding this comment

pljones Feb 24, 2022

Choose a reason for hiding this comment

pljones Feb 25, 2022

Choose a reason for hiding this comment

pljones Feb 25, 2022 • edited Loading

Choose a reason for hiding this comment

hoffie Feb 26, 2022

Choose a reason for hiding this comment

pljones Feb 27, 2022 • edited Loading

Choose a reason for hiding this comment

pljones Feb 23, 2022

Choose a reason for hiding this comment

pljones Feb 23, 2022

Choose a reason for hiding this comment

pljones commented Feb 23, 2022 • edited Loading

ann0see commented Feb 26, 2022 • edited Loading

pljones commented Feb 27, 2022 • edited Loading

softins left a comment

Choose a reason for hiding this comment

hoffie left a comment

Choose a reason for hiding this comment

pljones commented Feb 28, 2022 • edited Loading

ann0see commented Jul 25, 2022

pljones commented Jul 26, 2022 • edited Loading

pljones commented Feb 23, 2022 •

edited by ann0see

Loading

pljones Feb 23, 2022 •

edited

Loading

pljones Feb 25, 2022 •

edited

Loading

pljones Feb 27, 2022 •

edited

Loading

pljones commented Feb 23, 2022 •

edited

Loading

ann0see commented Feb 26, 2022 •

edited

Loading

pljones commented Feb 27, 2022 •

edited

Loading

pljones commented Feb 28, 2022 •

edited

Loading

pljones commented Jul 26, 2022 •

edited

Loading