Skip to content

Conversation

@furby-tm
Copy link

@furby-tm furby-tm commented Mar 15, 2023

Within this PR is support for:

  • minimum, basic dark mode support (toggle-able via the View menu in the menubar).
  • updated Swift schema for the replacement of Objective-C properties with the new Swift @Persisted property wrappers.

@cla-bot
Copy link

cla-bot bot commented Mar 15, 2023

Realm welcomes all contributions! The only requirement we have is that, like many other projects, we need to have a Contributor License Agreement (CLA) in place before we can accept any external code. Our own CLA is a modified version of the Apache Software Foundation’s CLA. Our records show that CLA has not been signed by @furby-tm. Please submit your CLA electronically using our Google form so we can accept your submissions. After signing the CLA you can recheck this PR with a @cla-bot check comment. The GitHub usernames you file there will need to match that of your Pull Requests. If you have any questions or cannot file the CLA electronically, make a comment here and we will be happy to help you out.

@furby-tm
Copy link
Author

@cla-bot check

@cla-bot cla-bot bot added the cla: yes label Mar 15, 2023
@cla-bot
Copy link

cla-bot bot commented Mar 15, 2023

The cla-bot has been summoned, and re-checked this pull request!

@furby-tm furby-tm changed the title Add basic support for dark mode. Add basic support for dark mode & update Swift schema. Mar 15, 2023
type: 'info',
message: `A new version of ${appName} is downloaded!`,
detail: `${appName} ${lastestVersion} is downloaded.\nClick "Ok" to quit and restart Realm Studio.`,
detail: `${appName} ${lastestVersion} is downloaded.\nClick "Ok" to quit and restart Cosmic Realms.`,
Copy link
Contributor

@kraenhansen kraenhansen Mar 16, 2023

Choose a reason for hiding this comment

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

Please revert this.

return str + 'Decimal128()';
return str;
case 'object':
return 'Objects must always be optional. Something is not right in this model!';
Copy link
Contributor

@kraenhansen kraenhansen Mar 16, 2023

Choose a reason for hiding this comment

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

I would prefer if the changes to the swift schema export went into a separate PR.


public toggleDarkMode()
{
const currentValue = this.shouldShowInternalFeatures();
Copy link
Contributor

@kraenhansen kraenhansen Mar 16, 2023

Choose a reason for hiding this comment

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

This line seems incorrect.

Comment on lines +13 to +14
"appId": "foundation.wabi.realm-studio",
"productName": "Cosmic Realms",
Copy link
Contributor

Choose a reason for hiding this comment

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

Please revert this.

// Post to Slack
postToSlack('slack-releases-webhook', [[
'title': "Realm Studio $VERSION has been released!",
'title': "Cosmic Realms $VERSION has been released!",
Copy link
Contributor

Choose a reason for hiding this comment

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

Please revert this.

{
"name": "realm-studio",
"productName": "Realm Studio",
"productName": "Cosmic Realms",
Copy link
Contributor

Choose a reason for hiding this comment

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

Please revert this.

"main": "./build/main.bundle.js",
"build": {
"appId": "com.mongodb.realm-studio",
"appId": "foundation.wabi.realm-studio",
Copy link
Contributor

Choose a reason for hiding this comment

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

Please revert this.

"protocols": [
{
"name": "Realm Studio",
"name": "Cosmic Realms",
Copy link
Contributor

Choose a reason for hiding this comment

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

Please revert this.

Comment on lines +5 to +6
const appBundleId = "foundation.wabi.realm-studio";
const ascProvider = "UQ9J5QT9DL"; // Apple: short team name
Copy link
Contributor

Choose a reason for hiding this comment

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

Please revert this.

});
*/
fs.writeFileSync(p, 'Hello from a future Realm Studio!', {
fs.writeFileSync(p, 'Hello from a future Cosmic Realms!', {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please revert this.

Comment on lines 3 to +35
"version": "999.0.0",
"lockfileVersion": 1,
"lockfileVersion": 3,
"requires": true,
"dependencies": {
"electron": {
"version": "file:../../../../node_modules/electron",
"packages": {
"": {
"name": "mocked-realm-studio",
"version": "999.0.0",
"license": "Apache-2.0",
"devDependencies": {
"electron": "file:../../../../node_modules/electron"
}
},
"../../../../node_modules/electron": {
"version": "19.1.8",
"integrity": "sha512-Bp1CwnYaIWXNL9ZjgEaLlEkVEGpjJMupcAnxyCe00C2ZhQT9gY+RJaPzkrZC+J/Gc4MvvLtJcy/Hvsc+MTkfNg==",
"dev": true
"dev": true,
"hasInstallScript": true,
"license": "MIT",
"dependencies": {
"@electron/get": "^1.14.1",
"@types/node": "^16.11.26",
"extract-zip": "^1.0.3"
},
"bin": {
"electron": "cli.js"
},
"engines": {
"node": ">= 8.6"
}
},
"node_modules/electron": {
"resolved": "../../../../node_modules/electron",
"link": true
Copy link
Contributor

Choose a reason for hiding this comment

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

Please revert this.

}

public changeTheme() {
this.setState({ darkModeEnabled: !this.state.darkModeEnabled });
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be persisted?

} else if (err.message === ARCHITECTURE_MISMATCH_MESSAGE) {
const improvedError = new Error(
'The file is already opened by another process, with an incompatible lock file format. Try up- or downgrading Realm Studio or SDK to match their versions of Realm Core.\n\nSee Realm Studio changelog on GitHub for details on compatibility between versions.',
'The file is already opened by another process, with an incompatible lock file format. Try up- or downgrading Cosmic Realms or SDK to match their versions of Realm Core.\n\nSee Cosmic Realms changelog on GitHub for details on compatibility between versions.',
Copy link
Contributor

Choose a reason for hiding this comment

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

Please revert this.

$realm-browser-header-icon-space: 24px;
$realm-browser-header-handle-hover-bg: rgba(0, 0, 0, .1);
$realm-browser-header-handle-dragging-bg: rgba($ultramarine, .5);
$realm-browser-header-handle-dragging-bg: rgba(#39477f, .5);
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this use a SCSS variable instead?

<head>
<meta charset="UTF-8">
<title>Realm Studio</title>
<title>Cosmic Realms</title>
Copy link
Contributor

Choose a reason for hiding this comment

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

Please revert this.

<head>
<meta charset="UTF-8">
<title>Realm Studio</title>
<title>Cosmic Realms</title>
Copy link
Contributor

Choose a reason for hiding this comment

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

Please revert this.

Comment on lines +98 to +110
@mixin themed() {
@each $theme, $map in $themes {
.theme--#{$theme} & {
$colors: () !global;
@each $key, $submap in $map {
$value: map-get(map-get($themes, $theme), '#{$key}');
$colors: map-merge($colors, ($key: $value)) !global;
}
@content;
$colors: null !global;
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you copy this from somewhere? If yes, please include a link.

label: "Enable Dark Mode",
id: 'toggle-appearance',
type: 'checkbox',
checked: store.shouldShowDarkMode(),
Copy link

@takameyer takameyer Mar 16, 2023

Choose a reason for hiding this comment

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

I would prefer if this was pulled from the system preferences. Would be great if there was a way to say "use my preferences" and then be able to provide an override.

@kraenhansen
Copy link
Contributor

kraenhansen commented Mar 16, 2023

@furby-tm could you post a few screenshots from the app when in dark mode?

@kraenhansen kraenhansen added the More-information-needed More information is needed to progress. The issue will close automatically in 2 weeks. label Mar 20, 2023
@furby-tm
Copy link
Author

Hey guys! Sorry, I'm just now getting to your response -- I've been tied down with some projects getting rolled out into production over the last couple weeks, so things have been hectic.

I had just a couple hours to spare, and spent that time trying to tone down the blinding light mode of the app, since I use it frequently, and figured my eyes could spare a break.

I realize my PR was a 0 context shove in, so I apologize for that!

To quickly summarize these changes:

I added a basic dark mode, and made some additional changes to the light mode which integrated some of the UI aspects of MongoDB Compass. The dark mode will need some cleanup since I missed a lot of the text on the tables (lots of it is looking black on black so that won't do!).

I brought in the latest changes to the realm swift generation schema, which is the removal of any/all objc boilerplate and integrated the ‘@persisted’ swift property wrappers.

And I believe that was all. If dark mode & rolling in the latest swift schematic changes are something the realm team are looking to support I'd be happy to fixup a proper PR for your review.

@github-actions github-actions bot removed the More-information-needed More information is needed to progress. The issue will close automatically in 2 weeks. label Mar 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants