Moon-0xff / gnome-mpris-label

A music related GNOME extension.
GNU General Public License v3.0
51 stars 10 forks source link

Add tooltips to Settings (Rebased) and update ChangeLog #21

Closed Batwam closed 1 year ago

Batwam commented 1 year ago

(Updated pull request rebased using latest main commits)

Included ability to include tooltips in Settings and started to populate some of them. image

Batwam commented 1 year ago

see proposed updates. I have broken them down as some may be more controversial than others...

Regarding the misaligned drop downs in the Label tab, I have included some comments. With the updates I made, they all align perfectly but your milage might vary...

before: image

after: image

Moon-0xff commented 1 year ago

I don't like the UI design changes submitted.

I don't like the reorder of the label section:

I reordered the widgets for aesthetic reasons, i feel they look nicer and tidier if the column of widgets (right column) is somewhat ordered by widget type and length.

I don't like the filters section ordered in two columns:

Setting the filters require typing, the entry boxes might be filled with a lot of text, for that reason the filters section benefits from wide entry boxes

Batwam commented 1 year ago

That's fine, feel free to pick and choose what works for you, these were mere suggestions, that's why I tried to split into separate commits.

Moon-0xff commented 1 year ago

It's my time to use git cherry-pick for the first time then

Batwam commented 1 year ago

Either that or git revert the changes you don't like so you can still keep the updates as a standalone branch prior to merging?

Moon-0xff commented 1 year ago

Done. The cherry pick bypassed the PR though.

For the record:

git remote add batwam git@github.com:Batwam/gnome-mpris-label.git
git fetch batwam
git branch prefs-changes
git switch prefs-changes
git cherry-pick batwam/tooltips_test~6
git cherry-pick batwam/tooltips_test~9
git cherry-pick batwam/tooltips_test~3 # no auto-merge, required some changes
git add prefs.js # to add the required edits
git cherry-pick --continue
git cherry-pick batwam/tooltips_test # for the last one
git add prefs.js # same thing as ~3
git cherry-pick --continue

I wanted to cherry-pick 401bef9 too, but required too much editing.

Moon-0xff commented 1 year ago

Merged to a new branch instead of main as there's still some changes to do. We can discuss them in the pull request for the branch (#22)

Batwam commented 1 year ago

I agree with the changes you picked and taking the opportunity to simplify some of the descriptions better explained by tooltip.

The only thing I would stil change is having the Reset button in the Filters tab should be half width for consistency with the other tabs? It's less consistent with the other buttons on the tab but at least it makes the Rest button look the same on all tabs. your call...