Skip to content
This repository has been archived by the owner on Jun 17, 2021. It is now read-only.

App settings modal #288

Closed
4 tasks done
AWolf81 opened this issue Oct 10, 2018 · 7 comments
Closed
4 tasks done

App settings modal #288

AWolf81 opened this issue Oct 10, 2018 · 7 comments
Labels
involves design Deals with visual stuff
Milestone

Comments

@AWolf81
Copy link
Collaborator

AWolf81 commented Oct 10, 2018

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):

  • Git settings (add later, skipped until we're working on Git integration)
  • Privacy settings: Enable/Disable analytics
  • General settings:
    • Default settings for project creation e.g. always start with a CRA app.
    • General: Language settings (overriding default picked language from OS langauge) once we're having i18n (issue Internationalization #66)

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

  • Add an application menu item to File\Settings... to display the new Component AppSettingsModal.
  • Create an app-settings.reducer to store the settings in Redux state
  • (optional UI button) Add a gear icon with SettingsButton & use settingsFor prop in SettingsButton handle click method (line 55) to render the app settings modal. I think we could add it to the bottom of the sidebar.

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,
  },
};
@AWolf81 AWolf81 added the involves design Deals with visual stuff label Oct 10, 2018
@mathieudutour
Copy link
Collaborator

mathieudutour commented Oct 10, 2018

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
Copy link
Collaborator Author

AWolf81 commented Oct 10, 2018

@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
Copy link
Owner

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:

  • Default project type
  • Enable/disable analytics

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
Copy link
Collaborator

Oh there is also the default project path

@AWolf81
Copy link
Collaborator Author

AWolf81 commented Oct 14, 2018

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
Copy link
Collaborator

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

@AWolf81 AWolf81 mentioned this issue Oct 16, 2018
7 tasks
@AWolf81 AWolf81 added this to the v0.4.0 milestone Oct 28, 2018
@AWolf81
Copy link
Collaborator Author

AWolf81 commented Nov 2, 2018

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.

@AWolf81 AWolf81 closed this as completed Nov 2, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
involves design Deals with visual stuff
Projects
None yet
Development

No branches or pull requests

3 participants