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

Settings Urls #649

Merged
merged 3 commits into from
May 11, 2023
Merged

Settings Urls #649

merged 3 commits into from
May 11, 2023

Conversation

Antolius
Copy link
Member

@Antolius Antolius commented May 10, 2023

Implements the Granite part of Settings Urls #190 issue from the docs repo.

Currently covers:

  • Location Services
  • Online Accounts
  • Network
  • App Permissions
  • Notifications
  • Sound Input
  • Custom Shortcuts

Links are showcased in a new demo app view as links:

Screenshot from 2023-05-11 21 56 29

Add string constants for URL links to settings pages from the Personal section. Add Settings URLs view to the Granite Demo app to showcase and test those links.
lib/Constants.vala Outdated Show resolved Hide resolved
lib/Constants.vala Outdated Show resolved Hide resolved
@danirabbit
Copy link
Member

Hey thanks for starting in on this! I left a couple responses to the comments you left previously.

Additionally I think we should probably keep this pretty tightly scoped to settings that we're sure are useful to developers so we don't end up with a bunch of extra un-useful constants to dig through. From the top of my head the things that seem useful are:

  • Location
  • Online accounts
  • Network
  • App permissions
  • Notifications?
  • Sound Input?

I'm not sure why, for example, an app other than Files would be linking to Housekeeping settings.

@Antolius
Copy link
Member Author

Additionally I think we should probably keep this pretty tightly scoped to settings that we're sure are useful to developers so we don't end up with a bunch of extra un-useful constants to dig through.

I must admit I went into this one with a bit of a completionist mindset. 😅

But yeah, we can definitely start with a narrow set of links that seem more useful, and potentially add new ones down the line if there's interest.

@Antolius
Copy link
Member Author

Antolius commented May 11, 2023

Ok, I checked the 3rd party apps in the AppCenter, and in addition to Nimbus requiring privacy, we have:

  1. Eddy linking to settings://privacy
  2. Clips linking to settings://input/keyboard/shortcuts
  3. Iridium linking to settings://network
  4. Stashed linking to settings://input/keyboard/shortcuts
  5. Home linking to settings://network

So yes, @lenemter, your pull request on keyboard settings is timed perfectly! I suggest adding settings://input/keyboard/shortcuts/custom to the list of constants.

Include just a selected set of deep Settings links and properly name-space them.
@Antolius Antolius marked this pull request as ready for review May 11, 2023 19:53
Copy link
Member

@danirabbit danirabbit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job! Reasonable list, I think. Works as expected :)

@danirabbit danirabbit merged commit a73e780 into elementary:main May 11, 2023
4 checks passed
@Antolius Antolius deleted the settings-urls branch May 20, 2023 20:31
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.

None yet

2 participants