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

[#368] add html as supported language #385

Merged
merged 11 commits into from
Nov 9, 2023
Merged

Conversation

d13ch
Copy link
Contributor

@d13ch d13ch commented Oct 3, 2023

  • Добавил HTML в список поддерживаемых языков;
  • Добавил иконку HTML;
  • Иконки отображаются в соответствии с языком сниппета (на странице сниппета и на карточках на странице профиля);
  • Сообщение "по умолчанию" в редакторе при создании сниппета отображается в соответствии с языком сниппета;
  • В зависимости от языка отображается терминал, либо блок превью для HTML;
  • Обновлены файлы локализации;

Дополнительно активировал уже подключенные языки.
Для полноценной работы необходимо, чтобы данные о языке приходили с сервера. На данном этапе для проверки работоспособности сделал подмену получаемых данных. Подмены сопроводил комментариями с тегом '#FIXME'.
На месте превью для HTML пока "пустышка".

@faciledictu
Copy link
Contributor

В assets не очень логично класть js-скрипт с функцией подбора иконки. Мне кажется более правильным решением код выбора иконки из файла frontend/src/pages/profile/NewSnippetForm.jsx
Его, при желании, можно вынести в отдельный модуль

@d13ch
Copy link
Contributor Author

d13ch commented Oct 9, 2023

Screenshot HTML Render

  • Блок превью отображает код из редактора

@d13ch
Copy link
Contributor Author

d13ch commented Oct 17, 2023

@fey @dzencot тут тесты ищут вывод в консоли, которой нет в HTML сниппете

@fey
Copy link
Contributor

fey commented Oct 17, 2023

А сейчас вывод консоли работает на JS? Может быть тесты поправить, чтобы они на JS ожидали вывод консоли, а для HTML - нет.

@d13ch
Copy link
Contributor Author

d13ch commented Oct 19, 2023

JS сниппеты я не трогал, так что всё должно работать, как работало.
Возможно, Елена сможет заняться тестами?

@d13ch
Copy link
Contributor Author

d13ch commented Oct 24, 2023

@fey я добавил тест для HTML. Его, наверное, лучше новым ПР в другой ветке оформить?

@fey
Copy link
Contributor

fey commented Oct 24, 2023

Лучше в этом

@d13ch
Copy link
Contributor Author

d13ch commented Oct 24, 2023

  • Убрал подмену языка сниппета в ответе сервера;
  • Добавил тест для HTML;

Приложение ожидает данные о языке сниппета в двух местах:

const getSnippetDataByViewParams = async ({ username, slug }) => {

export const fetchUserSnippets = createAsyncThunk(

@fey
Copy link
Contributor

fey commented Oct 24, 2023

Переведите пр в реди, если его можно ревьювить.

@fey fey requested review from dzencot and HelenOne October 24, 2023 12:16
@d13ch d13ch marked this pull request as ready for review October 24, 2023 15:48
@HelenOne
Copy link
Contributor

HelenOne commented Nov 2, 2023

Привет!
@d13ch а тесты playwright не должны проходить?

@d13ch
Copy link
Contributor Author

d13ch commented Nov 3, 2023

@HelenOne приветствую!
Нет, тест не пройдёт, пока не будет готов бэкенд

@geophyzik
Copy link
Contributor

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

 if (supportedLanguages.includes(language)) {
      try {

мб пора уже слайс подредактировать и оставить только js html

const slice = createSlice({
  name: 'languages',
  initialState: {
    supportedLanguages: ['javascript', 'php', 'python'],
    supportedLanguages: ['javascript', 'php', 'python', 'html'],
    currentLanguage: 'javascript',
  },
  reducers: {

@dzencot
Copy link
Collaborator

dzencot commented Nov 8, 2023

Добавил поддержку на бэкенде. Давайте мержить, если ручное тестирование ок. Тесты поправлю потом.

@dzencot dzencot merged commit db7c8e0 into hexlet-rus:main Nov 9, 2023
2 checks passed
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.

6 participants