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

fix: fix hint position bug EN-4,#9 #38 #83 #85 #109 #5

Merged
merged 9 commits into from
Jun 24, 2020
Merged

Conversation

AndVikVin
Copy link
Owner

@AndVikVin AndVikVin commented Sep 19, 2019

@AndVikVin AndVikVin requested a review from avasuro September 19, 2019 09:20
@@ -919,9 +920,28 @@ CanvasRenderingContext2D.prototype.roundRect = function(x, y, w, h, r) {
var bottom_offset = body_size.h - (data.center_y + half_h);
var left_offset = data.center_x - half_w;
var right_offset = body_size.w - (data.center_x + half_w);

Copy link
Collaborator

Choose a reason for hiding this comment

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

Этот алгоритм определения положения лейбла не достаточно хорошо определяет позиции куда лучше всего поставить label.
Например если лейбл очень узкий и высокий, а места сверху больше, чем места справа, но недостаточно для отображения лейбла (и при этом места справа хоть и меньше чем сверху, но достаточно чтоб отобразить лейбл) - то лейбл будет отображен по середине, хотя по логике должен быть отображен справа. Пример такой ситуации:

размеры лейбла: ширина=50 высота=500
Оффсеты:

//          400
//      ___________
//      |         |
//  60  |  20x200 |  20
//      |_________|
//          20

Прогони такой кейс по своему коду и увидишь что код будет пытаться разместить лейбл сверху, он не влезет и будет размещен по центру, хотя справа предостаточно места чтоб его отобразить.

Я могу предложить примерно такое решение (его к стати можно расширить чтоб и позицию по вертикали хорошо определять если надо):

// Массив возможных для размещения лейбла областей:
var areasForLabel = [
    {name: 'right', width: right_offset, height: window.innerHeight},
    {name: 'left', width: left_offset, height: window.innerHeight},
    {name: 'center', width: window.innerWidth, height: window.innerHeight}
];

// Определяем приоритеты по областям для размещения лейбла (от большей области к меньшей), типа:
// ['left', 'right']
var areasPriority = areasForLabel
    .sort((area1, area2) => area2.width - area1.width)
    .map(area => area.name);

// Сохраним величину пространства по горизонтали которое нужно лейблу для отображения:
var label_horizontal_space_required = label_width + label_margin;
var label_vertical_space_required = label_shift_with_label_height;

// Определяем где рендерить лейбл: идем по каждой стороне и пытаемся всунуть лейбл,
// eсли влазит - значит туда и станет.
// По умолчанию предполагается что лейбл вобще никуда не влезет (oversized):
var label_hor_side = 'oversized';
for (var i = 0; i < areasPriority.length; i++) {
    const areaName = areasPriority[i];
    const area = areasForLabel.find(area => area.name === areaName);
    if (area.width > label_horizontal_space_required && area.height > label_vertical_space_required) {
        label_hor_side = areaName;
        break;
    }
}

switch(label_hor_side) {
    case 'center':
        label_x = window.innerWidth / 2 - label_width / 2;
        break;
    case 'left':
        label_x = data.width ?
            data.center_x - label_width - data.width/2 - 20 :
            data.center_x - label_width - data.radius/2 - 20;
        break;
    case 'right':
        label_x = data.width ?
            data.center_x + data.width/2 + 20 :
            data.center_x + data.radius/2 + 20;
        break;
    case 'oversized':
        setTimeout(function(){
            $('.enjoy_hint_label').css({
                'border-radius': '40px',
                '-webkit-border-radius': '40px',
                '-moz-border-radius': '40px',
                'background-color': '#272A26',
                '-webkit-transition': 'background-color ease-out 0.5s',
                '-moz-transition': 'background-color ease-out 0.5s',
                '-o-transition': 'background-color ease-out 0.5s',
                'transition': 'background-color ease-out 0.5s'
            })
        }, 500)
        label_x = window.innerWidth / 2 - label_width / 2;
        break;
}


var areas_priority = areas_for_label
.sort(function(area1, area2){return area1.common_area - area2.common_area})
.map(function(area){return area.name});
Copy link
Collaborator

Choose a reason for hiding this comment

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

Не уверен на счет этого, зачем map? Потом в следующем блоке кода из-за него приходится делать find (строка 935)

if (area.width > label_horizontal_space_required && area.height > label_vertical_space_required) {
label_hor_side = area_name;
if(area_name === areas_priority[areas_priority.length - 1]) {
if (areas_priority[i].width > label_horizontal_space_required && areas_priority[i].height > label_vertical_space_required) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ну это ты уже перестарался - читаемость кода упала :) Оставь так как раньше было, исправь только одну строчку - var name = areas_priority[i]

if(area_name === areas_priority[areas_priority.length - 1]) {
if (areas_priority[i].width > label_horizontal_space_required && areas_priority[i].height > label_vertical_space_required) {
label_hor_side = areas_priority[i].name;
if(areas_priority[i].name === areas_priority[areas_priority.length - 1]) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

тут будет if(areas_priority[i].name === areas_priority[areas_priority.length - 1].name) { (name на конце забыл)

@AndVikVin AndVikVin merged commit 2fcbbfc into fixes Jun 24, 2020
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.

2 participants