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

attach: _multiple #1724

Closed
i2r opened this issue Nov 19, 2015 · 30 comments
Closed

attach: _multiple #1724

i2r opened this issue Nov 19, 2015 · 30 comments
Labels

Comments

@i2r
Copy link

i2r commented Nov 19, 2015

Есть потребность в аттаче, для которого будет работать множественное добавление файлов.

Есть два подхода к решению этой задачи.

  1. Добавить модификатор _multiple, при выставлении которого в true инпуту внутри кнопки будет добавлен атрибут multiple, а элемент с названием файла не будет отображаться совсем.
    При таком подходе мы сохраним обратную совместимость (+), но потеряем в гибкости настройки блока (-). С точки зрения кода, выглядеть это будет больше как костыль.

  2. Отрефакторить блок так, чтобы можно было настраивать два момента: а) множественное выделение, б) показ/скрытие имени файла (или количества выбранных файлов для множественного выбора). Так блок получится более гибким (+), но мы потеряем обратную совместимость (-/+) (хотя, есть вариант модификатора «не показывать название файла», тогда по умолчанию будет сохранено старое поведение). Код станет логичнее: есть контрол и два модификатора.

Ещё для второго варианта непонятно, как писать «2 файла», но «5 файлов», «21 файл» (ну и поддержка других языков).

Давайте обсудим, кто что думает по этому поводу.

@vitkarpov
Copy link

@tadatuta @veged

@tadatuta
Copy link
Member

Чуть подробнее раскрою, что подразумевается по рефакторингом:

  1. attach по умолчанию визуально становится просто кнопкой, без отображения выбранного файла.
  2. Текущее поведение будет достигаться модификатором _has-file.
  3. Новое поведение с _multiple, необходимое @i2r, предполагает отсутствие _has-file, но в целом стремимся к тому, чтобы модификаторы можно было сочетать.

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

@blond
Copy link
Member

blond commented Nov 19, 2015

@tadatuta, вам бы только семвер поломать :)

Откажитесь уже от v2, v3 ... vN веток в пользу dev или master, чтобы не ломать семвер. В bem-components же нету ничего кроме v2, в чём проблема каждый раз когда надо выпускать мажорную версию?

@veged
Copy link
Member

veged commented Nov 19, 2015

@blond давай не будем тут обсуждать отказ от v2, v3 ... vN веток, это оффтопик — можешь завести отдельный тикет, но позиция у нас по этому вопросу весьма упоротая устойчивая

@veged
Copy link
Member

veged commented Nov 19, 2015

я за промежуточный вариант, когда мы можем сохранить апи

проблему со склонением числа файлов решать не обязательно — мы можем просто выводить списком файлы

@i2r
Copy link
Author

i2r commented Nov 20, 2015

Для списка файлов нужно непредсказуемо много места: непонятно, как выводить — в строку или столбец, столбец над кнопкой или под?

Из всех браузеров только ИЕ выводит список, но в инпуте ограниченной ширины. И такой подход кажется неудобным, из-за того, что очень сложно понять, что же выбрано (даже пересчитать файлы в таком инпуте — проблема).

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

@i2r
Copy link
Author

i2r commented Nov 20, 2015

Склонение числа файлов можно оставить на совести пользователя, если дать возможность в bemJson передавать специальную строку с плейсхолдером, на место которого будет подставляться количество файлов, а возможные склонения указывать, например, через запятую.

@i2r
Copy link
Author

i2r commented Nov 20, 2015

API для второго варианта мог бы выглядеть так:

{
    block: 'attach',
    mods: {
        multiple: true,},

    /* Формат строки `legend`: 0 файлов, 1 файл, 2 файла, 5 файлов */
    legend: 'Файлы не выбран, $1 файл, $1 файла, $1 файлов',

    /* Алиас к `legend`, оставляем для обратной совместимости */
    noFileText: 'Файл не выбран',}

Соответственно, если строка legend не указана, то мы не показываем текст про выбранные файлы совсем (сейчас в этом случае рисуется пустой span, что выглядит как баг).

Так мы сохраняем совместимость, рефакторим аттач, избавляемся от бага с пустым спаном (для тех, у кого был указан noFileText, ничего не изменится), позволяем сочетать multiple с legend — PROFIT.

@dfilatov
Copy link
Member

А точно нужно писать прям количество выбранных файлов?

@i2r
Copy link
Author

i2r commented Nov 20, 2015

А точно нужно писать прям количество выбранных файлов?

Это способ показать, что инпут непустой (за пример взят стандартный контрол в Хроме, Сафари, ФФ). Для одиночного файла мы пишем название файла. На мой взгляд, делать длинный список имён всех выбранных файлов бессмысленно, так как пользоваться таким списком невозможно (см. как это делает IE) и для пользовтаеля оптимальнее станет отключить этот список и реализовать его самостоятельно. Т.е. мы потратим усилия на реализацию бесполезной фичи.

@alexeyten
Copy link

Я бы сделал API ещё проще и позволил добавить любой блок в который можно передать список файлов, а там пусть пользователь сам решает что он хочет показать, может количество файлов, а может предпросмотр картинок.

И в качестве примера реализовать блок который показывает имя файла или количество если их больше одного.

@alexeyten
Copy link

Хардкодить плейсхолдер плохо, непонятно как быть с разными языками.

@i2r
Copy link
Author

i2r commented Nov 20, 2015

@alexeyten в простейшем случае предлагает совсем отказаться от индикации того, что выбрано в аттаче. Для этого можно сделать новый блок, например, attach-simple, а на нынешний аттач отрефакторить так, что он внутри будет использовать attach-simple и уметь показывать выбранный файл (как сейчас).

При таком варианте мы точно не сломаем API нынешнего attach.

@voischev
Copy link
Member

file-name.png
2 файла
5 файлов
...
Такое?

@alexeyten
Copy link

Вот как раз из-за того, что всем от этого превью нужно разное, его нет большого смысла реализовывать тут.

@i2r
Copy link
Author

i2r commented Nov 20, 2015

Можно сделать так, что legend будет ожидать функцию, в которую будет передаваться список файлов из инпута, а возвращать эта функция должна строку, которую мы и покажем рядом с кнопкой.

@vitkarpov
Copy link

Нам точно нужна эта возможность, про легенду для множественных файлов?

На данный момент нужен блок attach, с модификатором multiple, который:

  • должен позволять выбирать несколько файлов
  • не должен ничего показывать рядом с кнопкой

Может стоит на данном этапе исходить из тех требований, которые есть? Или хочется заранее предусмотреть как этот блок должен работать с легендой?

@i2r
Copy link
Author

i2r commented Nov 20, 2015

Нам точно нужна эта возможность, про легенду для множественных файлов?

Только для сохранения API. Но можно сделать новый блок (attach-button?), тогда все проблемы отпадают.

@i2r
Copy link
Author

i2r commented Nov 22, 2015

3-й вариант: делаем новый блок — attach-button, который представляет собой кнопку и не пытается показывать ничего про файлы. На вход можно передать параметры аналогичные параметрам кнопки и multiple.
Так мы сохраняем API, получим простейший блок для прикрепления файлов, отображение выбранных файлов ложится полностью на плечи пользователя.

Мне больше всего нравится этот вариант.

@dfilatov
Copy link
Member

У нас, кстати, есть button_type_link, можно было бы по аналогии сделать button_type_attach, чтобы не плодить уж совсем новую сущность.

@voischev
Copy link
Member

Я против. Давайте тогда button_type_submit представим как input_type_submit => <input type="submit" value="Отправить">

так же с checkbox и radio поступим что бы не плодить новые сущности.

Кстати select это тоже тупо кнопка с попапом+меню. Давайте в button_type_select положим? :)

Абстракции над тем как предлагает голый html — это же хорошо. Не вижу ничего плохого в том что их больше чем есть по дефолту.

@voischev
Copy link
Member

а button_type_link — вообще что-то странное @pepelsbey на вас нет :)

@pepelsbey
Copy link
Contributor

@narqo
Copy link
Member

narqo commented Nov 24, 2015

Обсудили с @dfilatov лично: можно подумать над тем, чтобы переделать внутренности attach так: блок, в зависимости от модификатора, будет расчитывать на предоставление некоторого интерфейса приемника-файлов. В зависимости от конкретных потребностей, пользователь может реализовать свой сложный приемник и отдать его в наш attach (общая идея аналогична __switcher в dropdown). Это, в теории, позволи не ломать текущее API и сделать блок расширяемым. Про детали еще будем думать.

@i2r
Copy link
Author

i2r commented Nov 24, 2015

пользователь может реализовать свой сложный приемник и отдать его в наш attach

А зачем attach'у владеть таким приемником? Ну и мы получим ограничение на использование технологий для приемника — только bem.

@narqo
Copy link
Member

narqo commented Nov 24, 2015

А зачем attach'у владеть таким приемником?

В целом, идея похожа на описание в комменте #1724 (comment) но, должна позволить не тащить в библиотеку (очень) неоднозначные вещи вроде legend.

Ну и мы получим ограничение на использование технологий для приемника — только bem.

Кажется нет, никаких ограничений на то, как будет внутри работать приемник нет — главное чтобы он предоставлял для attach ожидаемый БЭМ-интерфейс (тут никаких отличий от текущих реалий).

@veged
Copy link
Member

veged commented Dec 3, 2015

может всё-таки сделаем самый простой случай с множественным выбором и отображением числа файлов (можно добавить попап со списком файлов)? а разные сложности можно будет делать доопределяя блок на своём уровне

@voischev
Copy link
Member

voischev commented Dec 3, 2015

@veged +1

@tadatuta
Copy link
Member

tadatuta commented Feb 8, 2016

@i2r Есть прогресс по задаче?

@veged veged added the v2 label Feb 8, 2016
@i2r
Copy link
Author

i2r commented May 27, 2016

Прогресса нет. Задача не актуальна.

@i2r i2r closed this as completed May 27, 2016
@deeonis deeonis removed the ready label May 27, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests