joshwcomeau / guppy

🐠A friendly application manager and task runner for React.js
ISC License
3.27k stars 154 forks source link

App settings modal #288

Closed AWolf81 closed 5 years ago

AWolf81 commented 5 years ago

We don't have an app settings modal yet but I think we should have one as we detected during #286.

We could add the following settings (some will require additional work):

I would group them into section so they're easier to find. I think a good start would be sections Git, Privacy & General.

Is your feature request related to a problem? Please describe. We'll face many situations where we require some user changable settings. See above to mention a few.

Describe the solution you'd like

Implementation details I'm working on branch app-settings. I'll push it soon. I'll add an AppSettings type like following to types (so we can use it later in App-settings.reducer):

export type AppSettings = {
  general: {
    defaultProjectPath: string,
    defaultProjcetType: string,
  },
  privacy: {
    trackingAllowed: boolean,
  },
};
mathieudutour commented 5 years ago

Another important settings is opting out of the analytics.

I don’t think we need any icon on the UI. It’s pretty well established convention to have the preferences on the top bar menu (at least on macOS)

AWolf81 commented 5 years ago

@mathieudutour Good idea. I've added it to the list above. And you're right just the application menu item is enough for accessing the settings.

joshwcomeau commented 5 years ago

I'm not sure we need to add an auto-commit setting just yet... I added some details in #286, but for ejecting specifically, I think it's safe to assume that if a user is looking to eject, they're an intermediate-advanced user and we can just ask them to get a Git client.

In the future I'd like for Guppy to support rudimentary Git operatons (#85), but for now I think we can outsource it.

I agree with all of @mathieudutour's feedback :)

Without Git, there's actually not many settings I can think of:

It'll be a pretty sparse menu for now, but I think that's OK. Analytics is super important, and project type is a convenient thing to have.

I agree with your planned implementation, having an app-settings reducer.

mathieudutour commented 5 years ago

Oh there is also the default project path

AWolf81 commented 5 years ago

OK, I'm work on this and I'll only add Privacy setting & General settings with default project path & default project type.

For the settings item location. I've seen on Mac there is a Menu item called Guppy (see screenshot below). It's probably the menu with role services. See docs here. VS code is adding the app settings there.

Should we use it or is it better to be consistent on every platform and add it always to File/Preferences...?

grafik

mathieudutour commented 5 years ago

Every macOS app have it under the app name menu so let's put it there has well

AWolf81 commented 5 years ago

We can close this as the app settings are available on master.

We just had to fix the path issue for Mac. PR #321 from @melanieseltzer with the fix is ready.

Issue for refactoring of the AppSettingsModal render not created yet. I have an idea how it could be refactored so it will automatically render the modal based on a settings configuration file.