Skip to content

Commit

Permalink
feat(windows): Customizable title bar style (#3765)
Browse files Browse the repository at this point in the history
__Issue:__ There are several quirks on Windows with the 'undecorated' Windows style - issues like:
- #3730 
- #3071 
- #3063 

__Fix:__ These can be deal-breakers for use as a daily editor, so until we have those fixed for the 'undecorated' Windows, change the default setting to use the native titlebar. 

This introduces a new setting - `window.titleBarStyle` that can be `"native"` or `"custom"`. The `"custom"`  titlebar looks nicer, because it is themed and custom rendered, however it has the above quirks. On Windows, change the default to `"native"`.

In addition, this pushes up the configuration loading sooner in the startup cycle, so we can pick up configuration settings like `"window.titleBarStyle"` prior to opening the window.

__Todo:__
- [x] Fix margin on Windows, based on whether we are using the decorated window or not
- [x] Test OSX
- [x] Test Windows (both settings)
- [x] Test on Linux (both settings)
  • Loading branch information
bryphe authored Jul 22, 2021
1 parent 8e0298e commit 0cd8c42
Show file tree
Hide file tree
Showing 12 changed files with 150 additions and 42 deletions.
4 changes: 4 additions & 0 deletions CHANGES_CURRENT.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@
- #3718 - Completion: Add `editor.suggest.itemsToShow` setting (fixes #3712)
- #3736 - Search: add default keys to go to next / previous search result (fixes #3713)
- #3733 - Quick Open: Add bindings to open in splits, not current buffer.
- #3765 - UX: Add `"window.titleBarStyle"` configuration setting

> __BREAKING:__ On Windows, the default setting is to use the `"native"` title bar.
> Set `"window.titleBarStyle": "custom"` to keep the previous behavior.
### Bug Fixes

Expand Down
1 change: 1 addition & 0 deletions bench/lib/Helpers.re
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ let simpleState = {
~titlebarHeight=0.,
~getZoom=() => 1.0,
~setZoom=_zoom => (),
~useNativeTitleBar=true,
);

Reducer.reduce(
Expand Down
2 changes: 2 additions & 0 deletions docs/docs/configuration/settings.md
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,8 @@ The configuration file, `configuration.json` is in the Oni2 directory, whose loc

- `window.menuBarVisibility` __(_"visible" | "hidden"_ default: `"visible"`)__ - Controls the visibility of the menu bar.

- `window.titleBarStyle` __(_"native" | "custom"_ default: `"native"` on Windows, `"custom"` otherwise)__ - Controls whether the titlebar is custom-rendered.

- `oni.layout.showLayoutTabs` __(_"always"|"smart"|"never"_ default: `"smart"`)__ - Controls the display of layout tabs. `"smart"` will only show the tabs if there's more than one.

- `oni.layout.layoutTabPosition` __(_"top"|"bottom"_ default: `"bottom"`)__ - Controls the position of the layout tabs.
Expand Down
1 change: 1 addition & 0 deletions integration_test/lib/Oni_IntegrationTestLib.re
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ let runTest =
~titlebarHeight=0.,
~setZoom,
~getZoom,
~useNativeTitleBar=true,
),
);

Expand Down
2 changes: 2 additions & 0 deletions src/Feature/Configuration/Feature_Configuration.rei
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ module ConfigurationLoader: {
let none: t;

let file: FpExp.t(FpExp.absolute) => t;

let loadImmediate: t => result(Config.Settings.t, string);
};

let initial:
Expand Down
34 changes: 34 additions & 0 deletions src/Feature/Configuration/GlobalConfiguration.re
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,16 @@ module Decoders = {
),
),
]);

let titleBarStyle: decoder([ | `Native | `Custom]) =
string
|> map(String.lowercase_ascii)
|> map(
fun
| "native" => `Native
| "custom" => `Custom
| _ => `Custom,
);
};

module Encoders = {
Expand All @@ -57,6 +67,11 @@ module Encoders = {
| `None => string("none")
| `Inline => string("inline")
};

let titleBarStyle: encoder([ | `Native | `Custom]) =
fun
| `Native => string("native")
| `Custom => string("custom");
};

module Codecs = {
Expand Down Expand Up @@ -292,6 +307,24 @@ module Search = {
let followSymlinks = setting("search.followSymlinks", bool, ~default=true);
};

module Window = {
// On Windows, default the titlebar style to native, due to various bugs around the custom-rendered window, like:
// #3730 - Keyboard shortcuts for window movement broken
// #3071 - Window has no shadow on Windows
// #3063 - Part of onivim fullscreen is visible on second monitor
let defaultTitleBarStyle =
switch (Revery.Environment.os) {
| Windows(_) => `Native
| _ => `Custom
};
let titleBarStyle =
setting(
"window.titleBarStyle",
custom(~decode=Decoders.titleBarStyle, ~encode=Encoders.titleBarStyle),
~default=defaultTitleBarStyle,
);
};

module Workbench = {
let activityBarVisible =
setting("workbench.activityBar.visible", bool, ~default=true);
Expand Down Expand Up @@ -319,6 +352,7 @@ let contributions = [
Editor.snippetSuggestions.spec,
Files.exclude.spec,
Explorer.autoReveal.spec,
Window.titleBarStyle.spec,
Workbench.activityBarVisible.spec,
Workbench.editorShowTabs.spec,
Workbench.editorEnablePreview.spec,
Expand Down
26 changes: 20 additions & 6 deletions src/Feature/TitleBar/Feature_TitleBar.re
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,17 @@ type msg =
| WindowCloseClicked
| TitleDoubleClicked;

type model = {
// We need to store this, as opposed to pulling it
// from config, because we need to respect the value
// used on initialization.
useNativeTitleBar: bool,
};

let isNative = ({useNativeTitleBar, _}) => useNativeTitleBar;

let initial = (~useNativeTitleBar) => {useNativeTitleBar: useNativeTitleBar};

module Log = (val Log.withNamespace("Oni2.Feature.TitleBar"));

module Internal = {
Expand Down Expand Up @@ -155,7 +166,7 @@ type outmsg =
| Nothing
| Effect(Isolinear.Effect.t(msg));

let update = (~maximize, ~minimize, ~restore, ~close, msg) => {
let update = (~maximize, ~minimize, ~restore, ~close, msg, model) => {
let internalDoubleClickEffect =
Isolinear.Effect.create(~name="window.doubleClick", () => {
switch (Internal.getTitleDoubleClickBehavior()) {
Expand All @@ -174,11 +185,11 @@ let update = (~maximize, ~minimize, ~restore, ~close, msg) => {
Isolinear.Effect.create(~name="window.restore", () => restore());

switch (msg) {
| TitleDoubleClicked => Effect(internalDoubleClickEffect)
| WindowCloseClicked => Effect(internalWindowCloseEffect)
| WindowMaximizeClicked => Effect(internalWindowMaximizeEffect)
| WindowRestoreClicked => Effect(internalWindowRestoreEffect)
| WindowMinimizeClicked => Effect(internalWindowMinimizeEffect)
| TitleDoubleClicked => (model, Effect(internalDoubleClickEffect))
| WindowCloseClicked => (model, Effect(internalWindowCloseEffect))
| WindowMaximizeClicked => (model, Effect(internalWindowMaximizeEffect))
| WindowRestoreClicked => (model, Effect(internalWindowRestoreEffect))
| WindowMinimizeClicked => (model, Effect(internalWindowMinimizeEffect))
};
};

Expand Down Expand Up @@ -537,11 +548,13 @@ module View = {
~theme,
~font: UiFont.t,
~height,
~model,
(),
) => {
let title =
title(~activeBuffer, ~workspaceRoot, ~workspaceDirectory, ~config);
switch (Revery.Environment.os) {
| Mac(_) when isNative(model) => React.empty
| Mac({major, _}) =>
<Mac
isFocused
Expand All @@ -555,6 +568,7 @@ module View = {
height
majorVersion=major
/>
| Windows(_) when isNative(model) => menuBar
| Windows(_) =>
<Windows
menuBar
Expand Down
12 changes: 10 additions & 2 deletions src/Feature/TitleBar/Feature_TitleBar.rei
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@ type windowDisplayMode =
| Maximized
| Fullscreen;

type model;

let initial: (~useNativeTitleBar: bool) => model;

let isNative: model => bool;

[@deriving show]
type msg;

Expand All @@ -32,9 +38,10 @@ let update:
~minimize: unit => unit,
~restore: unit => unit,
~close: unit => unit,
msg
msg,
model
) =>
outmsg;
(model, outmsg);

// VIEW

Expand All @@ -54,6 +61,7 @@ module View: {
~theme: ColorTheme.Colors.t,
~font: UiFont.t,
~height: float,
~model: model,
unit
) =>
Revery.UI.element;
Expand Down
3 changes: 3 additions & 0 deletions src/Model/State.re
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,7 @@ type t = {
modal: option(Feature_Modals.model),
snippets: Feature_Snippets.model,
textContentProviders: list((int, string)),
titleBar: Feature_TitleBar.model,
vim: Feature_Vim.model,
zoom: Feature_Zoom.model,
autoUpdate: Feature_AutoUpdate.model,
Expand All @@ -516,6 +517,7 @@ let initial =
~titlebarHeight,
~getZoom,
~setZoom,
~useNativeTitleBar,
) => {
let config =
Feature_Configuration.initial(
Expand Down Expand Up @@ -668,6 +670,7 @@ let initial =
quickOpen: Feature_QuickOpen.initial,
snippets: Feature_Snippets.initial,
terminals: Feature_Terminal.initial,
titleBar: Feature_TitleBar.initial(~useNativeTitleBar),
textContentProviders: [],
vim: Feature_Vim.initial,
zoom: Feature_Zoom.initial(~getZoom, ~setZoom),
Expand Down
24 changes: 14 additions & 10 deletions src/Store/Features.re
Original file line number Diff line number Diff line change
Expand Up @@ -2542,21 +2542,25 @@ let update =
);

| TitleBar(titleBarMsg) =>
let (titleBar', outmsg) =
Feature_TitleBar.update(
~maximize,
~minimize,
~close,
~restore,
titleBarMsg,
state.titleBar,
);
let eff =
switch (
Feature_TitleBar.update(
~maximize,
~minimize,
~close,
~restore,
titleBarMsg,
)
) {
switch (outmsg) {
| Feature_TitleBar.Effect(effect) => effect
| Feature_TitleBar.Nothing => Isolinear.Effect.none
};

(state, eff |> Isolinear.Effect.map(msg => TitleBar(msg)));
(
{...state, titleBar: titleBar'},
eff |> Isolinear.Effect.map(msg => TitleBar(msg)),
);

| ExtensionBufferUpdateQueued({triggerKey}) =>
let maybeBuffer = Selectors.getActiveBuffer(state);
Expand Down
11 changes: 8 additions & 3 deletions src/UI/Root.re
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ module Constants = {
module Styles = {
open Style;

let root = (theme, windowDisplayMode) => {
let root = (~nativeTitleBar, theme, windowDisplayMode) => {
let style =
ref([
backgroundColor(Colors.Editor.background.from(theme)),
Expand All @@ -33,7 +33,9 @@ module Styles = {
justifyContent(`Center),
alignItems(`Stretch),
]);
if (Revery.Environment.isWindows && windowDisplayMode == State.Maximized) {
if (Revery.Environment.isWindows
&& windowDisplayMode == State.Maximized
&& !nativeTitleBar) {
style := [margin(6), ...style^];
};
style^;
Expand Down Expand Up @@ -252,7 +254,9 @@ let make = (~dispatch, ~state: State.t, ()) => {
// Correct for zoom in title bar height
let titlebarHeight = state.titlebarHeight /. zoom;

<View style={Styles.root(theme, state.windowDisplayMode)}>
let nativeTitleBar = Feature_TitleBar.isNative(state.titleBar);

<View style={Styles.root(~nativeTitleBar, theme, state.windowDisplayMode)}>
<Feature_TitleBar.View
menuBar=menuBarElement
activeBuffer=maybeActiveBuffer
Expand All @@ -267,6 +271,7 @@ let make = (~dispatch, ~state: State.t, ()) => {
dispatch=titleDispatch
registrationDispatch
height=titlebarHeight
model={state.titleBar}
/>
<View style=Styles.workspace>
<View style=Styles.surface>
Expand Down
Loading

0 comments on commit 0cd8c42

Please sign in to comment.