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

feat: remove plugin order rendering config (z-index) #761

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,6 @@ When several plugins create decorations for the same layer, the general rule is
- On the foreground layer, the `highlight-over` decorations are rendered before the `highlight-outine` decorations.
- When two plugins adds the same type of decorations at the same place, the decorations are rendered in the order they have been created.

But fortunately, it's possible to reorder decorations on their specific layer using these settings. These settings works as a `z-index` in CSS, the higher the value, the higher the decorations will be in the render stack, for instance, a plugin's decorations with an order value of `1` will appear above decorations from a plugin with an order value of `0`.

#### Smooth Scrolling

Whether to offset the minimap canvas when scrolling to keep the scroll smooth. When `true` the minimap canvas will be offseted, resulting in a smoother scroll, but with the side-effect of a blurry minimap when the canvas is placed between pixels. When `false` the canvas will always stay at the same position, and will never look blurry, but the scroll will appear more jagged. `(default=true)`
Expand Down
15 changes: 1 addition & 14 deletions lib/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -253,17 +253,6 @@ export function onDidDeactivatePlugin (callback) {
return emitter.on('did-deactivate-plugin', callback)
}

/**
* Registers a callback to listen to the `did-change-plugin-order` event of
* the package.
*
* @param {function(event:Object):void} callback the callback function
* @return {Disposable} a disposable to stop listening to the event
*/
export function onDidChangePluginOrder (callback) {
return emitter.on('did-change-plugin-order', callback)
}

/**
* Returns the `Minimap` class
*
Expand Down Expand Up @@ -388,7 +377,6 @@ const MinimapServiceV1 = {
onDidRemovePlugin,
onDidActivatePlugin,
onDidDeactivatePlugin,
onDidChangePluginOrder,
minimapClass,
minimapForEditorElement,
minimapForEditor,
Expand All @@ -400,8 +388,7 @@ const MinimapServiceV1 = {
togglePluginActivation: PluginManagement.togglePluginActivation,
deactivateAllPlugins: PluginManagement.deactivateAllPlugins,
activatePlugin: PluginManagement.activatePlugin,
deactivatePlugin: PluginManagement.deactivatePlugin,
getPluginsOrder: PluginManagement.getPluginsOrder
deactivatePlugin: PluginManagement.deactivatePlugin
}

/**
Expand Down
5 changes: 0 additions & 5 deletions lib/minimap-element.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { CompositeDisposable, Disposable } from 'atom'
import { EventsDelegation, AncestorsMethods } from 'atom-utils-plus'
import elementResizeDetectorImport from 'element-resize-detector'

import * as Main from './main'
import CanvasDrawer from './mixins/canvas-drawer'
import include from './decorators/include'
import element from './decorators/element'
Expand Down Expand Up @@ -707,10 +706,6 @@ class MinimapElement {
this.pendingFrontDecorationChanges.push(change)
}
this.requestUpdate()
}),

Main.onDidChangePluginOrder(() => {
this.requestForcedUpdate()
})
)

Expand Down
7 changes: 1 addition & 6 deletions lib/mixins/canvas-drawer.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import { escapeRegExp } from '../deps/underscore-plus'
import Mixin from 'mixto'

import * as Main from '../main'
import { domStylesReader } from '../main'
import CanvasLayer from '../canvas-layer'

Expand Down Expand Up @@ -149,7 +148,7 @@ export default class CanvasDrawer extends Mixin {
lineHeight,
charWidth,
charHeight,
orders: Main.getPluginsOrder()
orders: {}
}

const drawCustomDecorationLambda = (decoration, data, decorationColor) => drawCustomDecoration(decoration, data, decorationColor, editorElement)
Expand Down Expand Up @@ -777,10 +776,6 @@ function drawDecorations (screenRow, decorations, renderData, types, editorEleme
)
}

decorationsToRender.sort((a, b) =>
(renderData.orders[a.properties.plugin] || 0) - (renderData.orders[b.properties.plugin] || 0)
)

if (decorationsToRender != null ? decorationsToRender.length : undefined) {
for (let i = 0, len = decorationsToRender.length; i < len; i++) {
const decoration = decorationsToRender[i]
Expand Down
51 changes: 2 additions & 49 deletions lib/plugin-management.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,6 @@ export const plugins = {}
*/
const pluginsSubscriptions = {}

/**
* A map that stores the display order for each plugin
*
* @type {Object}
* @access private
*/
const pluginsOrderMap = {}

/**
* Registers a minimap `plugin` with the given `name`.
*
Expand All @@ -63,7 +55,7 @@ export function registerPlugin (name, plugin) {
emitter.emit('did-add-plugin', event)

if (atom.config.get('minimap.displayPluginsControls')) {
registerPluginControls(name, plugin)
registerPluginControls(name)
}

updatesPluginActivationState(name)
Expand Down Expand Up @@ -192,9 +184,8 @@ export function deactivatePlugin (name, plugin) {
* to toggle the plugin state.
* @access private
*/
function registerPluginControls (name, plugin) {
function registerPluginControls (name) {
const settingsKey = `minimap.plugins.${name}`
const orderSettingsKey = `minimap.plugins.${name}DecorationsZIndex`

const config = getConfigSchema()

Expand All @@ -205,59 +196,21 @@ function registerPluginControls (name, plugin) {
default: true
}

config.plugins.properties[`${name}DecorationsZIndex`] = {
type: 'integer',
title: `${name} decorations order`,
description: `The relative order of the ${name} plugin's decorations in the layer into which they are drawn. Note that this order only apply inside a layer, so highlight-over decorations will always be displayed above line decorations as they are rendered in different layers.`,
default: 0
}

if (atom.config.get(settingsKey) === undefined) {
atom.config.set(settingsKey, true)
}

if (atom.config.get(orderSettingsKey) === undefined) {
atom.config.set(orderSettingsKey, 0)
}

pluginsSubscriptions[name].add(atom.config.observe(settingsKey, () => {
updatesPluginActivationState(name)
}))

pluginsSubscriptions[name].add(atom.config.observe(orderSettingsKey, (order) => {
updatePluginsOrderMap(name)
const event = { name, plugin, order }
emitter.emit('did-change-plugin-order', event)
}))

pluginsSubscriptions[name].add(atom.commands.add('atom-workspace', {
[`minimap:toggle-${name}`]: () => {
togglePluginActivation(name)
}
}))

updatePluginsOrderMap(name)
}

/**
* Updates the display order in the map for the passed-in plugin name.
*
* @param {string} name the name of the plugin to update
* @access private
*/
function updatePluginsOrderMap (name) {
const orderSettingsKey = `minimap.plugins.${name}DecorationsZIndex`

pluginsOrderMap[name] = atom.config.get(orderSettingsKey)
}

/**
* Returns the plugins display order mapped by name.
*
* @return {Object} The plugins order by name
*/
export function getPluginsOrder () { return pluginsOrderMap }

/**
* When the `minimap.displayPluginsControls` setting is toggled,
* this function will unregister the commands and setting that
Expand Down
4 changes: 1 addition & 3 deletions spec/minimap-element-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -350,8 +350,6 @@ describe('MinimapElement', () => {

expect(calls).toEqual(['bar', 'foo'])

atom.config.set('minimap.plugins.fooDecorationsZIndex', -1)

calls.length = 0
})

Expand All @@ -362,7 +360,7 @@ describe('MinimapElement', () => {
runs(() => {
nextAnimationFrame()

expect(calls).toEqual(['foo', 'bar'])
expect(calls).toEqual(['bar', 'foo'])

Main.unregisterPlugin('foo')
Main.unregisterPlugin('bar')
Expand Down
2 changes: 0 additions & 2 deletions spec/minimap-main-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -178,12 +178,10 @@ describe('Minimap package', () => {

it('creates a default config for the plugin', () => {
expect(minimapPackage.getConfigSchema().plugins.properties.dummy).toBeDefined()
expect(minimapPackage.getConfigSchema().plugins.properties.dummyDecorationsZIndex).toBeDefined()
})

it('sets the corresponding config', () => {
expect(atom.config.get('minimap.plugins.dummy')).toBeTruthy()
expect(atom.config.get('minimap.plugins.dummyDecorationsZIndex')).toEqual(0)
})

describe('triggering the corresponding plugin command', () => {
Expand Down