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

Feature/quick settings custom element #269

Merged
merged 3 commits into from Feb 16, 2015

Conversation

abe33
Copy link
Contributor

@abe33 abe33 commented Feb 15, 2015

No description provided.

@quickSettingsElement.destroy()
@quickSettingsSubscription.dispose()
else
MinimapQuickSettingsElement ?= require './minimap-quick-settings-element'

Choose a reason for hiding this comment

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

Line exceeds maximum allowed length

@fundon
Copy link
Member

fundon commented Feb 15, 2015

I had make a post on Atom'discuss.
Maybe it helps us to fetch some important informations.
https://discuss.atom.io/t/minimap-new-feature-tester-needed/14919

@fundon
Copy link
Member

fundon commented Feb 15, 2015

@abe33 see https://github.com/fundon/atom-minimap/blob/feature/quick_settings_custom_element/lib/minimap-element.coffee#L279

    @applyStyles @controls,
      width: Math.min(@canvas.width, @width) + 'px'

The code should be replaced with:

    @applyStyles @controls,
      width: Math.min(@canvas.width / devicePixelRatio, @width) + 'px'

If the window.devicePixelRatio is not 1, the canvas.width will be x2 (window.devicePixelRatio = 2) https://github.com/fundon/atom-minimap/blob/feature/quick_settings_custom_element/lib/minimap-element.coffee#L345,

    if canvasWidth isnt @canvas.width or @height isnt @canvas.height
      @canvas.width = canvasWidth * devicePixelRatio

but the controls div does not need width x 2.

I think that this issue's point.

@abe33
Copy link
Contributor Author

abe33 commented Feb 15, 2015

The devicePixelRatio is much more legit than the previous solution. I'll see how I can cover retina screen in some tests. There's probably a need for some helpers method to doesn't have to multiply/divide the width explicitely every time we make a computation.

minimum: 1
maximum: 3
default: window.devicePixelRatio
description: 'The device pixel ratio used to draw the canvas on high-DPI device.'

Choose a reason for hiding this comment

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

Line exceeds maximum allowed length

@fundon
Copy link
Member

fundon commented Feb 15, 2015

Yep, detects devicePixelRatio is needful.

@fundon
Copy link
Member

fundon commented Feb 15, 2015

I added minimap.devicePixelRatio setting for high-DPI device.
Now, everything looks good.

@fundon fundon force-pushed the feature/quick_settings_custom_element branch from 3b2a4b0 to 7909b5c Compare February 16, 2015 06:43

canvasTop = @minimap.getFirstVisibleScreenRow() * @minimap.getLineHeight() - @minimap.getScrollTop()

canvasTransform = @makeTranslate(0, canvasTop)
canvasTransform += " " + @makeScale(1/devicePixelRatio) if devicePixelRatio isnt 1
canvasTransform += " " + @makeScale(1 / devicePixelRatio) if devicePixelRatio isnt 1

Choose a reason for hiding this comment

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

Line exceeds maximum allowed length

@fundon
Copy link
Member

fundon commented Feb 16, 2015

Ok, I removed the setting.
I'm not sure that we really need it.

@abe33 abe33 merged commit c276d4f into master Feb 16, 2015
@fundon
Copy link
Member

fundon commented Feb 16, 2015

Yeah, it's released! 👍

@abe33 abe33 deleted the feature/quick_settings_custom_element branch February 16, 2015 23:26
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

3 participants