PROMO-251(OG): WIP: Add text-overflow for title #410
Conversation
| } | ||
|
|
||
| function getTextWidth(text, options) { | ||
| return ((options.fontSize * 400) / options.fontWeight) * 0.9 * text.length; |
There was a problem hiding this comment.
Тут надо наоборот делить =)
|
P.S На breadcrumbs не смотри, они в следующих PRах |
| textToSVG.loadSync(resolve(template.path, template.name, template.params.font)), | ||
| ); | ||
| // trunc file extension | ||
| const fontName = template.params.font.slice(0, -4); |
There was a problem hiding this comment.
| ]; | ||
|
|
||
| // prevent errors while parsing XML svg object | ||
| function isolateXMLSpecifiedSymbols(text) { |
There was a problem hiding this comment.
suggestion(non-blocking): Мб что-то типа function escapeText?
There was a problem hiding this comment.
непонятно звучит конечно, но давай escapeText! =)
There was a problem hiding this comment.
Ну тип "экранирование" ~ "чистка"
@GhostMayor Как бы такое назвал?)
| } else svg += SVGText(filteredText, { ...options, name: font.name }); | ||
| svg += `</svg>`; |
There was a problem hiding this comment.
nitpick: Лучше сразу обработать негативный сценарий и выбросить значение из функции
А оставшееся тело функции куда будет проще и ацикломатически, и в плане читаемости
| }); | ||
| } | ||
|
|
||
| console.error(filteredText, reformat.result, reformat.temp); |
There was a problem hiding this comment.
nitpick: Если консолим, то хотя бы уточни что именно)
Особенно если ошибка (хотя непонятно почему ошибка именно должна быть, если тут все валидно)
| svg += font.declaration; | ||
|
|
||
| if (getTextWidth(text, options) > widthLimit) { | ||
| const reformat = filteredText.split(" ").reduce( |
There was a problem hiding this comment.
nitpick: Некрит, но лучше при разбиении отдельно константу words завести. Почище будет, и можно к ней обращаться отдельно в цикле
| if (getTextWidth(`${acc.temp} ${cur}`, options) < widthLimit) { | ||
| return { ...acc, temp: `${acc.temp} ${cur}` }; | ||
| } | ||
|
|
||
| return { | ||
| temp: cur, | ||
| result: [...acc.result, acc.temp], | ||
| }; | ||
| }, | ||
| { | ||
| temp: "", | ||
| result: [], | ||
| }, | ||
| ); | ||
| svg += SVGText(reformat.temp, { | ||
| ...options, | ||
| name: font.name, | ||
| }); | ||
| for (let i = reformat.result.length - 1; i >= 0; i--) { | ||
| svg += SVGText(reformat.result[i], { | ||
| ...options, | ||
| transform: `${options.transform} translate(0, ${ | ||
| (reformat.result.length - i) * textPaddingTop | ||
| })`, | ||
| name: font.name, | ||
| }); | ||
| } |
There was a problem hiding this comment.
issue: А вот это дело прям упростить надо... Т.к. оч сложно считывается, и прям велика вероятность потом где-то тут ошибку сделать
suggestion: Как варианты, что в голову приходят:
- Свести все к генерации массива строк
lines(опять же по критерию< widthLimit)
2.A. (в идеале) Переписать логику стилей так, чтобы каждый объект можно было обернуть спокойно вSVGText(...)и не шаманить в циклах
2.B. Если же так нельзя (а учитывая наши замашки по шаблону - так рили может быть) - то хотя бы привести к простому циклу, мб тому же редьюсу - где в константуcontentSvgредьюсится итоговое значение с учетом переноса строк - И как итог - просто вставляем в return template-string нужные значения
contentSvg,font.declaration- и гораздо чище получится
There was a problem hiding this comment.
На самом деле, оч смущает столько хелперов и этапов шаманств с текстом, но учитывая изначальные требования - мб по другому и никак... @feature-sliced/contributors @feature-sliced/core мб будут мнения как еще проще в svgшке текст перенести?
There was a problem hiding this comment.
На самом деле я не представляю как ты тут это всё разбирал, магию редьюсов и тд, т.к. это всё в качестве черного варианта было накидано, чтоб вообще расмотреть идею с самописным svg. Кстати owerflow через foreignObject не предлагать, он его не понимает =((
| return ((options.fontSize * 400) / options.fontWeight) * 0.9 * text.length; | ||
| } | ||
|
|
||
| function textToSVG(font, text, options, widthLimit = 1300, textPaddingTop = -70) { |
|
спасибо что поревьюил =) PR немного WIP был, извиняюсь, что не пометил в заголовке, меня интересовали именно сомнительные на мой взгляд решения. Потому там всякие консоль логи не выпелены и ошибки не исправлены с делением и тд. )) и марафет не наведён |
Кста если вдруг будут идеи сюда впихнуть остальные ишьюсы (например с breadcrumbs)- то лучше не надо) И так PR пухленький вышел |
Спасибо не надо, я уже научен горьким опытом вливания месяцами =))) он у меня давно в другой ветке)) |
Krakazybik
changed the title
CHANGELOG
Добавлено:
Удалено:
REFERENCE
#401 #251
Чеклист