Conversation
|
Однією з хороший якостей фронтенд розробника є не тягнути бібліотеки в проект, якщо в них немає реальної необхідності. У тебе тут цілий білд ангуляра тільки для того щоб показати пару рядків тексту на сторінці. Зроби це все без ангуляра. Все що тут потрібно це тільки rxjs і 20 рядків коду, якщо не менше |
|
сюди навіть вебпака не потрібно. Ціль даної домашки просто навчитись працювати з лібою rxjs без прив'язки до ангуляра, бо це просто ліба, яку ангуляр використовує, а не щось особливе ангуляра |
Я думав, що у нас в рамках розпочатого курсу з ангуляра тепер все йде з його використанням. Нiяким разом не пропагую iдею роздувати проекти, сам такого не люблю(в git MA я навмисно не пушу весь проект, а тiльки пару невеликих файлiв. Саме з метою не додавати лишьного). До виконання поставленого завданя з боку RxJs якiсь зауваження/покращення потрiбнi? |
|
Також, менi цiкаво було погратися на практицi з changeDetection та деякими аспектами пов'язаними саме з ангуляром |
| .subscribe(this.wordHandler); | ||
| } | ||
|
|
||
| wordHandler(keyboardEvents: KeyboardEvent[]) { |
There was a problem hiding this comment.
по факту все що тобі потрібно це поле key з івента. Оскільки івентів буде дууже багато, то краще оптимізовувати це діло і ще на самому початку відкидати непотрібні дані і працювати тільки з необхідним
| buffer( | ||
| this.keyUp$.pipe( | ||
| filter(isSpace) | ||
| ) | ||
| ) |
There was a problem hiding this comment.
особисто мені не дуже подобаються такі конструкції.
this.keyUp$.pipe(
filter(isSpace)
)тут є логіка того як відбираються дані для буіеризації стріма. це незалежний стрім і його можна винести в окрему змінну наверху з своєю назвою. Так буде зрозуміліше з першого погляду що цей стрім робить і не буде в коді ялиночок з дужок
| return event.code === 'Space'; | ||
| } | ||
|
|
||
| function not(fn) { |
There was a problem hiding this comment.
якщо все пишеш на ts то типізація має бути скрізь. на всіх аргументах і типи які функції повертає
| .cursor { | ||
| display: inline-block; | ||
| transform: translateY(-2px); | ||
| margin-left: -2px; |
There was a problem hiding this comment.
щоб не гратись з розміщенням тексту можна для дівайдерів (в даному випадку це вертикальний дівайдер) використовувати бордер.
| if (!changes.word.currentValue) return; | ||
| this.text += this.word + ' '; |
There was a problem hiding this comment.
ченж детекшин для такого прям слішком. просто закинь новий https://developer.mozilla.org/en-US/docs/Web/API/Document/createTextNode в дом і забудь про нього. і не потрібно нічякого ченж детекшина і перевірок, і не буде з часом кілометрової стрінги в пам'яті
| } | ||
|
|
||
| function isSpace(event: KeyboardEvent) { | ||
| return event.code === 'Space'; |
There was a problem hiding this comment.
можна перевіряти по event.key === ' ' і в тебе буде використовуватись тільки одна пропертя івента і не треба буде тягнути весь об'єкт івента через стрім
| this.currentWord = ''; | ||
| keyboardEvents.forEach(({ key }) => { | ||
| this.currentWord += key; | ||
| }); |
There was a problem hiding this comment.
якщо юзати потрібні методи, то код буде простішим
this.currentWord = keyboardEvents.map(event => event.key);| keyUp$: Observable<Event> = null; | ||
| keyUpSubscribtion: Subscription = null; | ||
|
|
||
| constructor(public cd: ChangeDetectorRef) { |
There was a problem hiding this comment.
public cd: ChangeDetectorRef хорошим тоном є робити публічними тільки ті поля, які потрібно. Всі інші, особливо якщо на цьому зав'язана внутрішня реалізація класу, краще робити приватними. Там коли хтось буде використовувати цей клас, то він буде чітко розуміти що можна викристовувати, а що ні
| if (!keyboardEvents.length) return; | ||
| this.currentWord = ''; | ||
| keyboardEvents.forEach(({ key }) => { | ||
| this.currentWord += key; |
There was a problem hiding this comment.
keyboardEvents.forEach(({ key }) => {
this.currentWord += key;А ця конструкція хіба не буде купу раз трігерити ченж детекшин ангуляра?
2 participants
Demo: https://andreyzagoruy-dev.github.io/ng-writer/dist/ng-writer/