Informacja o dawno zadanym pytaniu

[wizarddos]

Ta zmiana jest z issue #293
Dodałem prostą informację otoczoną czerwoną ramką. Pojawia się gdy użytkownik otworzy za pomocą przycisku formularz odpowiedzi, a przy pytaniach bez odpowiedzi, po prostu domyślnie się pokazuje

Kwestią do ustalenia może być jeszcze kolor ramki, oraz treść informacji

A tak prezentuje się ramka

Jasny Motyw:

Ciemny Motyw:

[event15]

"(...) i może nie być już aktualne" albo "(...) i może być już nieaktualne"

[ScriptyChris]

Na oko, kontrast czcionki względem tła (grafitowy vs beżowy?) jest zbyt mały na jasnym motywie. Może białe tło byłoby lepsze dla tego koloru czcionki?

[event15]

Moje approve zawsze tyczą się tylko i wyłącznie PHP

[ScriptyChris]

Wcześniej to przeoczyłem, myśląc że placeOfOutdatedQuestionInfo jest wyżej pobrane z DOM tylko po to, aby przekazać go zaraz do funkcji showInfoAboutOutdatedQuestion. Ale skoro ta zmienna jest używana w tym warunku, to jednak lepiej przekazać ją do funkcji, niż tam pobierać drugi raz to samo - czyli poprzednia wersja była ok.

Aczkolwiek, żeby nie commitować takiej w sumie mało istotnej zmiany osobno, to IMO lepiej ewentualnie przyłączyć ją do jakichś pozostałych - o ile będą.

[ScriptyChris]

O ile kontrast tekstu względem tła na jasnym motywie nie okaże się zbyt mały, to jak dla mnie jest ok.

[awaluk]

Wg mnie ten tekst, padding od niego do ramki jak i margines pod ramką są stanowczo zbyt duże. Dobrze, że zwraca to na siebie uwagę, ale bez przesady. Na małych ekranach jak telefon jest jeszcze gorzej, bo ten komunikat np. u mnie zajmuje jakieś mniejsze pół ekranu.

Gdy ktoś kliknie "odpowiedz", pojawia się ta informacja, a następnie kliknie się "anuluj" to informacja dalej pozostaje, wydaje się, że wtedy powinna też znikać.

Poza tym ok, faktycznie pomyślałbym jeszcze tylko nad ustaleniem tekstu w ramce. "To pytanie zostało zadane ponad 2 miesiące temu" brzmi jak dla mnie myląco w sytuacji, gdy mamy pytanie, które zostało zadane np. rok temu - można to dwuznacznie odebrać. Jak już to zmieniłbym ten początek na coś w rodzaju "To pytanie zostało zadane już dawno temu". A w drugim zdaniu jest wtedy ciągle powtórzone słowo "pytanie". Można by spróbować więc zmienić całość, może np. na coś w rodzaju "To pytanie zostało zadane już dawno temu i może być nieaktualne. Upewnij się, że Twoja odpowiedź nadal będzie pomocna."

[awaluk]

To już jest gotowe do sprawdzenia? Bo wydaje mi się, że opisane rzeczy w większości nadal są

[wizarddos]

Teraz jest wszystko. Nie wrzuciłem tu wczoraj jednego z commitów. Teraz już jest

[ScriptyChris]

Można to przypisanie zrobić przy pomocy ternarki:

const shouldBeDisplayed = placeOfOutdatedQuestionInfo.classList.contains('hidden') ? 'hidden' : '';

Poza tym, nazwa zmiennej sugeruje, że to boolean, a nie string z nazwą klasy, więc to bym poprawił.

[ScriptyChris]

Jeśli nie ma potrzeby obliczania stylu dynamicznie, to lepiej jest manipulować klasą niż ustawiać styl inline.

[ScriptyChris]

Dużą literą powinno nazywać się klasy lub funkcje-konstruktory, a DOM element to jest zwykły obiekt, więc lepiej jest użyć camelCase.

[ScriptyChris]

Z tego co widzę w DOM inspektorze na forum, to inputy [name=docancel] służą za czerwony przycisk anulujący pisanie komentarza (lub odpowiedzi, jeśli jest już jakaś udzielona). Jeśli więc jest potrzeba złapania przycisku anulującego aktualnie pisany post, to raczej na pewno nie można tego tak zahardkodować.

Można wyciągnąć wszystkie te inputy (przyciski) i w pętli podpiąć im event listener, albo podpiąć się pod aktualnie aktywną instancję CKEditora i relatywnie od niej znaleźć przycisk anulujący; jakbyś miał z tym problem, to daj znać, pomogę (w trakcie developmentu dla ułatwienia można sobie wrzucić console.log(CKEDITOR.currentInstance) w paro sekundowy setTimeout, żeby móc zfocusować edytor na czas sprawdzania instancji - inaczej to property zwróci null). Ewentualnie zmatchować najbliższego przodka edytora i przycisku anulującego na podstawie klucza z obiektu CKEDITOR.instances.

[awaluk]

Jeśli chodzi o funkcjonalność, to po dodaniu odpowiedzi komunikat nadal pozostaje, a nie ma już sensu, aby tam był. Do tego wchodząc jako niezalogowany na pytanie bez odpowiedzi widoczny jest od razu komunikat, co też nie ma sensu, bo niezalogowany nawet nie ma jak odpowiedzieć. No i jak dla mnie ten rozmiar czcionki nadal jest sporo za duży, wystarczyłoby lekkie zwrócenie uwagi, co już robi czerwona ramka, a nie info na pół ekranu :)

[wizarddos]

Można wyciągnąć wszystkie te inputy (przyciski) i w pętli podpiąć im event listener

Coś w tym stylu?

const buttons = document.querySelectorAll('button');

buttons.for_each(button =>{
     if(button.name === "docancel"){
        button.addEventListener('click', ()=>{
            // kod zmieniający widoczność
        });
    }
});

albo podpiąć się pod aktualnie aktywną instancję CKEditora i relatywnie od niej znaleźć przycisk anulujący

Z tym rozwiązaniem mam problem. Na razie nie mogę dobrze zrozumieć dokumentacji i znaleźć w niej jak mogę 'złapać' buttony z dołu

Jeśli chodzi o funkcjonalność, to po dodaniu odpowiedzi komunikat nadal pozostaje, a nie ma już sensu, aby tam był.

Tak się teraz zastanawiam. Dlaczego to ma znikać? Rozumiem gdy są odpowiedzi, a użytkownik nie chce odpowiadać, lub wchodzi ktoś nie zalogowany, Ale gdy ktoś odpowiada, to może to chyba zawsze widzieć?

[ScriptyChris]

Coś w tym stylu?

Nie ma potrzeby robić ifa w pętli, skoro można elementy wyfiltrować poprzez selektor:

const cancelButtons = document.querySelectorAll('[name="docancel"]');

cancelButtons.forEach((cancelBtn) => {
  cancelBtn.addEventListener('click', () => {
    // kod zmieniający widoczność
  });
});

Z tym rozwiązaniem mam problem. Na razie nie mogę dobrze zrozumieć dokumentacji i znaleźć w niej jak mogę 'złapać' buttony z dołu

Poniżej przykład kodu, który łapie przycisk do anulowania posta - setTimeout jest tylko do celów deweloperskich, żeby móc zfocusować edytor (inaczej nie złapie aktualnej instancji). Dodałem też filmik, w którym widać jak to działa. Otworzyłem trzy instancje CKEditora (dla komentarza do pytania, komentarza do odpowiedzi i do samej odpowiedzi) i spośród wielu instancji można dostać się do aktualnej:

// setTimeout(() => {
    const currentCKEInstanceElement = CKEDITOR.currentInstance.element.$;
    console.log('currentCKEInstanceElement:', currentCKEInstanceElement, '\n/cke instance name:', currentCKEInstanceElement.name);

    const closestCKEInstanceForm = currentCKEInstanceElement.form;
    const cancelButton = closestCKEInstanceForm.docancel;

    console.log('cancelButton:', cancelButton);
// }, 1000);

2022-05-05.13-07-42.mp4

---

---

Tylko teraz tak sobie myślę, że nie wiem czy jest sens łapać aktualną instancję edytora, bo przecież użytkownik wcale nie musi mieć jej zfocusowanej w danym momencie. Można by więc po prostu złapać wszystkie przyciski [name="docancel"], w pętli podpiąć im listenery i na klik ukryć komunikat relatywny/najbliższy przyciskowi - tak jak do tej pory togglujesz klasę "hidden" (tyle, że bezpieczniej by było ją eksplicytnie dodawać zamiast togglować). Analogicznie w przypadku pokazania komunikatu - usunąć klasę "hidden", zamiast ją togglować.

Ponadto, myślę że też przydało by się obsłużyć przypadek pokazania wielu komunikatów, a nie jednego - bo przecież użytkownik może próbować dodać komentarze do różnych odpowiedzi i pytania oraz samą odpowiedź i przy każdej instancji edytora będzie pokazywany osobny komunikat, który wypada zamknąć po kliknięciu w przycisk "Anuluj".

Przy okazji, czy zostało już ustalone, że komunikat pokazuje się dla odpowiedzi i komentarzy, czy tylko odpowiedzi? Bo, jeśli to ma dotyczyć obu przypadków, to trzeba by uelastycznić ten skrypt.

Na oko, przerobiłbym go w ten sposób:

const allPostSendingButtons = document.querySelectorAll(
  'form[name*="_form"] input:is([value="Odpowiedz"], [value="Skomentuj"])'
); // weź wszystkie przyciski zatwierdzające dodanie posta
allPostSendingButtons.forEach((postSendingButton) => {
  postSendingButton.addEventListener('click', possiblyShowOutdatedInfo);
});

function possiblyShowOutdatedInfo({ target: postSendingButton }) {
  if (!isOutdatedInfoNeeded()) {
    return;
  }

  const postFormContainer = postSendingButton.form.parentElement;
  prepareToHideOutdatedInfo(postSendingButton.form.docancel, postFormContainer);

  // to jest chyba zbędne, bo skoro kontener jest ukryty, to i komunikat będzie ukryty
  const outdatedInfoMaybeHidden = postFormContainer.style.display === 'none' ? "hidden" : "";

  postFormContainer.insertAdjacentHTML(
    'beforebegin', 
    `<p class = "qa-outdated-question-container ${outdatedInfoMaybeHidden}">
      To pytanie zostało zadane już dawno temu i może być nieaktualne.<br/>
      Upewnij się, że Twoja odpowiedź nadal będzie pomocna.
    </p>`
  );
}

function prepareToHideOutdatedInfo(cancelBtn, postFormContainer) {
  cancelBtn.addEventListener('click', () => {
    // skoro komunikat został wstawiony jako 'beforebegin', to można założyć, że jest poprzednim rodzeństwem kontenera
    // chociaż pytanie, czy nie lepiej usunąć komunikat niż go ukrywać? Bo co jeśli użytkownik znowu zechce dodać post - wtedy zobaczy zduplikowany komunikat?
    postFormContainer.previousElementSibling.classList.add('hidden');
  }, { once: true });
}

function isOutdatedInfoNeeded() {
    const publishDateSpan = document.querySelector('.published > .value-title');
    const now = new Date();
    const publishDate = new Date(publishDateSpan.title);
    const publishYearOlderThanNow = publishDate.getFullYear() < now.getFullYear();
    const publishMonthNewerThanNow = publishDate.getMonth() - 1 >= now.getMonth();

    return publishYearOlderThanNow && publishMonthNewerThanNow;
}

[wizarddos]

No i jak dla mnie ten rozmiar czcionki nadal jest sporo za duży, wystarczyłoby lekkie zwrócenie uwagi, co już robi czerwona ramka, a nie info na pół ekranu :)

Niby racja, ale spójrzmy jeszcze na jedną rzecz. Gdy dodajemy pytanie w kategorii programowanie i nie wstawimy bloczka pokazuje nam się takie okienko

Jest ono średniej wielkości i zwraca uwagę, ale pomimo tego jest naprawdę sporo pytań, w których jest kod ale bez bloczka. To był główny powód, dlaczego ta informacja jest tak duża

Przy okazji, czy zostało już ustalone, że komunikat pokazuje się dla odpowiedzi i komentarzy, czy tylko odpowiedzi? Bo, jeśli to ma dotyczyć obu przypadków, to trzeba by uelastycznić ten skrypt.

Ostatecznie to jeszcze nie. Wg. mnie powinniśmy podłączyć to tylko dla odpowiedzi, bo tylko odpowiedź odkopuje pytanie. To właśnie jest główny cel tej ramki, w jakimś stopniu zapobiegać odkopywaniu pytań.

[awaluk]

Tak się teraz zastanawiam. Dlaczego to ma znikać? Rozumiem gdy są odpowiedzi, a użytkownik nie chce odpowiadać, lub wchodzi ktoś nie zalogowany, Ale gdy ktoś odpowiada, to może to chyba zawsze widzieć?

Pomyślałem o znikaniu, bo ta informacja po dodaniu odpowiedzi już nic nie da, a wręcz ktoś może nie wiedzieć o co z nią chodzi. W końcu miał formularz odpowiedzi, postanowił np. zignorować komunikat i jednak dodać odpowiedź, bo np. uznał, że będzie ona pomocna i skoro już dodał odpowiedź i nic więcej nie pisze, to nie wiem po co nadal go ostrzegać, że temat może być przeterminowany. Jedyne co mi przychodzi do głowy to aby mógł się zreflektować i jednak odpowiedź usunąć, ale skoro juz ją napisał... Poza tym jak wtedy odświeży stronę to i tak zniknie. Więc nie wiem, zdawało mi się, że ten napis powinien być zawsze związany z momentem pisania odpowiedzi.

Jest ono średniej wielkości i zwraca uwagę, ale pomimo tego jest naprawdę sporo pytań, w których jest kod ale bez bloczka. To był główny powód, dlaczego ta informacja jest tak duża

Trafna uwaga, ale obawiam się, że tu okienko żadnej wielkości nic nie da. Że część osób jest po prostu bardzo oporna i nie zwróci uwagi na żadne okienko, bo stwierdzi, że to ich nie dotyczy, że nie wiedzą jak to zrobić albo że wiedzą lepiej, że należy wstawić właśnie tak. No ale ok, jeśli tak to można dać to okienko trochę większe niż dotyczące bloczka kodu, ale nadal mam wątpliwości czy aż tak duże. Tym bardziej w kontekście mniejszych ekranów.

Ostatecznie to jeszcze nie. Wg. mnie powinniśmy podłączyć to tylko dla odpowiedzi, bo tylko odpowiedź odkopuje pytanie. To właśnie jest główny cel tej ramki, w jakimś stopniu zapobiegać odkopywaniu pytań.

Teoretycznie komentarz też jakoś odkopuje pytanie, ale fakt faktem wtedy nie wybija się na górę strony głównej, a tylko na "ostatnia aktywność". No i spora różnica jest tu w znaczeniu odpowiedzi, która ma wprost odpowiadać na dany problem (co może nie mieć sensu jak ktoś o coś zapytał rok temu), a komentarzem (który może służyć np. tylko do tego, aby dopytać kogoś po roku o to czy mu podane w odpowiedzi rozwiązanie działa). Inna rzecz, że ludzie czasem to mylą... :) Jak dla mnie możemy na razie zostać przy odpowiedziach, a najwyżej kiedyś się to rozwinie jeśli uznamy, że jest potrzeba.

[wizarddos]

Dobra, wszystko powinno już działać jak należy.
Przy okazji poprawiłem wyświetlanie się informacji u osób nie zalogowanych (wcześniej się wyświetlało, teraz już nie)

Czekam na review i ewentualne poprawki

[ScriptyChris]

Wystarczy przypisać podwójną negację (dwa wykrzykniki):

const areThereAnswersForQuestion = !!CKEDITOR.instances.a_content;

[ScriptyChris]

Dlaczego tutaj .toggle() zostało zamienione na ręczne sprawdzanie obecności klasy i odpowiednio jej usunięcie lub dodanie? Przecież do tego właśnie służy .toggle(). :)

[wizarddos]

To w końcu co znaczy "eksplicytnie dodawać". Wspominałeś o tym, i tak to zinterpretowałem. Czyli wcześniejsze tooglowanie było dobrze?

[ScriptyChris]

Ah, zapomniałem, że o tym pisałem wyżej. Chodziło mi o to, żeby na kliknięcie w cancelAnswer dodać klasę 'hidden', a na kliknięcie w '#q_doanswer' ją usunąć - wtedy eksplicytnie określa się, że anulowanie odpowiedzi ukrywa komunikat, a kliknięcie w dodanie odpowiedzi, pokazuje komunikat. Togglowanie mogło by przez pomyłkę zrobić odwrotnie i jest troszkę mniej czytelnie, jeśli się spojrzy na to od strony intencji.

Czyli po prostu:

cancelAnswer.addEventListener('click', () => outdatedInfoContainerClassList.add('hidden'));

document.querySelector('#q_doanswer').addEventListener('click', () => {
            outdatedInfoContainerClassList.remove('hidden');

           // reszta kodu
});

Jeszcze tak ogólnie, sprawdziłbym, czy listenery będą nadal działać, jeśli:

1. klikniesz w dodanie odpowiedzi
2. klikniesz w anuluj
3. znowu klikniesz w dodanie odpowiedzi
4. znowu klikniesz w anulowanie

, bo tam korzystasz z opcji { once: true }, więc listener wtedy zadziała tylko raz.

[wizarddos]

Ok, dzięki za rozjaśnienie

A co do tych listenerów, wszystko u mnie działała jak powinno. once: true jest tam po to, aby listener dodał się raz, nie więcej. Bez tego listenery dodawały się za każdym razem, gdy kliknąłem przycisk doanswer kod wywoływał się więcej niż 1 raz. W tej wersji wszystko działa poprawnie

[ScriptyChris]

Jak wyżej

[wizarddos]

Poprawione

[awaluk]

Funkcjonalnie zdaje się, że wszystko jest ok, o ile komunikat celowo nie ukrywa się po dodaniu odpowiedzi i tak po prostu ma być, wyżej o tym wspominałem.

Nadal uważam, że komunikat jest za wielki na mniejszych ekranach, u mnie na telefonie zajmuje jakąś 1/3 strony.

I jeszcze jedna rzecz nieco poboczna mi wpadła: jak mamy pytanie, które ma wiele komentarzy do pytania i klikamy pod jego treścią (a więc nad komentarzami) przycisk "odpowiedz" to następuje otwarcie formularza odpowiedzi (na dole, pod komentarzami) i automatyczne przewinięcie strony do niego. Problem jest jednak taki, że przewinięcie następuje do samego edytora, a więc pod ramkę z komunikatem, można wtedy jej zwyczajnie nie zauważyć. Chyba lepiej jakby scrollowało do początku ramki, aby ona była od razu widoczna.

[ScriptyChris]

Nie .toggle('hidden') tylko .remove('hidden') - eksplicytnie usuwamy klasę (pokazując tym samym komunikat) na kliknięcie w przycisk dodający odpowiedź. Tak jak pokazałem w komentarzu.

[wizarddos]

Nie jestem pewny czy to mogłoby zadziałać. Formularz odpowiedzi możemy zamknąć również przyciskiem otwierania odpowiedzi, a nie tylko czerwonym anuluj. Toggle jest tu aby nie było jasno narzucone, którym przyciskiem mamy zamykać formularz

[ScriptyChris]

Faktycznie, przycisk "Odpowiedz" toggluje formularz z edytorem - nie zauważyłem. Więc .toggle('hidden') może zostać.

[ScriptyChris]

Nie .toggle('hidden') tylko .add('hidden') - eksplicytnie dodajemy klasę (ukrywając tym samym komunikat) na kliknięcie w przycisk anulujący odpowiedź. Tak jak pokazałem w komentarzu.

[ScriptyChris]

Czy nie brakuje tutaj drugiej negacji - !!CKEDITOR.instances.a_content? W obecnej formie, jeśli a_content jest falsy (czyli np. puste pole edytora z odpowiedzią), to w areThereAnswersForQuestion będzie true (bo jest pojedyncza negacja), a chyba chcemy odwrotnie. Pisałem o tym w komentarzu.

[wizarddos]

Czy nie brakuje tutaj drugiej negacji

Nie nie brakuje. jeżeli od razu jest wyświetlana instancja CKEditora to wyrażenie zwraca false, ponieważ nie ma przycisku anuluj, więc po co dodawać event listenera do czegoś co nie istnieje?
Trochę się tutaj jednak pomyliłem przy nazywaniu. Powinno to być bardziej coś w stylu cancelButtonExist.

[ScriptyChris]

Ok.

[wizarddos]

problem jest jednak taki, że przewinięcie następuje do samego edytora, a więc pod ramkę z komunikatem, można wtedy jej zwyczajnie nie zauważyć.

Naprawiłem to. Zmniejszyłem też lekko tekst.

[awaluk]

Coś to przewijanie do edytora nie do końca działa, przynajmniej u mnie na Chrome. Na standardowym full HD zostaje sporo miejsca nad komunikatem, choć jest na tyle miejsca, aby jeszcze przewinąć w dół, strona się nie kończy. A im mniejszy ekran, tym jakby się przewijało coraz niżej i w efekcie na bardzo małym przewija się gdzieś na środek edytora. Poniżej filmik na którym to widać. Myślałem, że może to Q2A ma takie problemy samo w sobie, ale potestowałem też na produkcji i tam wygląda dobrze.

2022-06-24.18-52-14.mp4

[wizarddos]

Naprawiłem tego scrolla na full HD ale na ten moment nie jestem w stanie poprawić tego scrollowania na mniejszych ekranach
Przy moim debugowaniu znalazłem coś co może być tego przyczyną

Na samym początku mój kod scrolluje do ramki z informacją i wszystko wygląda dobrze, gdy jednak edytor się załaduje to scroll idzie jeszcze dalej, zakładam przez użycie czegoś w rodzaju window.scroll() (nie jestem tego pewien, jeszcze nie znalazłem kodu odpowiedzialnego za to w oryginałach CKEditora)

[ScriptyChris]

Na samym początku mój kod scrolluje do ramki z informacją i wszystko wygląda dobrze, gdy jednak edytor się załaduje to scroll idzie jeszcze dalej, zakładam przez użycie czegoś w rodzaju window.scroll() (nie jestem tego pewien, jeszcze nie znalazłem kodu odpowiedzialnego za to w oryginałach CKEditora)

Możesz do window lub document.documentElement spróbować podpiąć MutationObserver obserwujący zmianę atrybutów i jeśli on złapie moment tego niewiadomego scrollowania, to wtedy ustaw breakpoint i powinno przekierować Cię do kodu, który to powoduje.

Albo skorzystaj z breakpointa łapiącego scroll (z tego co widzę on jest w grupie Control -> scroll).