Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mg new features #71

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Mg new features #71

wants to merge 5 commits into from

Conversation

matg22
Copy link

@matg22 matg22 commented Feb 20, 2023

Hi,
My name is Mathieu, I'm French, product manager but former developer (java). I love coding and love doing it listening to music.
I'm using your extension since several years now and I implemented additional features a long time ago. Finally, I recently found time to do some cleaning and to propose something to you.

Those additional features mainly consists in:

  • displaying title of the album
  • introducing preferences (use app name 'Extensions' to get/set preferences) to customize the current display.
  • adding an inline indicator (the title and the playback status is displayed in the top bar).

A lot of theses features may be activated or deactivated through preference panel, so your extension may keep its actual look'n feel if you're not interested in using them.

I'm beginner to gjs development so feel free to say anything negative or positive to this work. If I need to rework some part, do not hesitate to tell me.
I hope you'll like it.

All the best,
Mathieu

@JasonLG1979
Copy link
Owner

JasonLG1979 commented Mar 1, 2023

Sorry about the delay.

displaying title of the album

I'm OK with this the only caveat is that the album cover size is determined by the combined height of the text boxes so unless the overall width of the main widget was set appropriately the text would end up having less horizontal space. As soon as someone finishes/fixes this PR by adding a style class to define the max-width so it works as intended we can do that, or if you would like to fix that issue and roll in the album title I'm perfectly OK with that too.

introducing preferences (use app name 'Extensions' to get/set preferences) to customize the current display.

Preferences of any kind are a non-starter, never going to happen. Once you add one you eventually end up with a hundred. The general idea is to just have relatively sane defaults.

adding an inline indicator (the title and the playback status is displayed in the top bar).

This is also a non-starter and will never happen. There is no way to know what else could be occupying panel. The last thing anyone needs is this extension crowding other extensions. There is a tooltip for this reason.

@matg22
Copy link
Author

matg22 commented Apr 4, 2023

Sorry about the delay.
Ok, I understand your answers even if I don't agree with you but that's your point of view and you are the boss here.

I'm going to rework the album info addition. For the two others features (preferences, inline indicator), I will maintain them on my own.

@JasonLG1979
Copy link
Owner

Sorry about the delay. Ok, I understand your answers even if I don't agree with you but that's your point of view and you are the boss here.

I'm going to rework the album info addition. For the two others features (preferences, inline indicator), I will maintain them on my own.

Sounds good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants