Skip to content

Informacja o dawno zadanym pytaniu #297

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.

Sign up for GitHub

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

Draft
wants to merge 11 commits into
base: master
Choose a base branch
from
Draft

Informacja o dawno zadanym pytaniu #297

Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
fac6f34
initial commit
wizarddos Apr 11, 2022
1c3a13e
Feature - Added valid info about question
wizarddos Apr 12, 2022
657a33a
Fix - Refactor according to pr suggestions
wizarddos Apr 13, 2022
dd46a93
Fix - fix according to pr suggestions
wizarddos Apr 24, 2022
fa67d49
Fix-added missing event listener to cancel button
wizarddos Apr 26, 2022
afce8f1
Merge branch 'CodersCommunity:master' into fetaure/outdated-question-…
wizarddos Jun 16, 2022
b3ea532
Refactor - refactor according to pr suggestions
wizarddos Jun 16, 2022
8d57fa0
Refactor - refactor according to pr suggestions
wizarddos Jun 17, 2022
e6348bb
Refactor - Refactor according to pr suggestions
wizarddos Jun 23, 2022
ea2ee2d
Refactor - added some spaces+ removed 1 style
wizarddos Jun 23, 2022
80ec452
Fix - Fixed scrolling on full HD in chrome
wizarddos Sep 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions forum/qa-plugin/outdated-question-info/frontend/show-outdated-question-info.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,16 @@ if(placeOfOutdatedQuestionInfo){
if(questionElemExist){
const outdatedInfoContainerClassList = questionElemExist.classList;

const areThereAnswersForQuestion = !CKEDITOR.instances.a_content ? true : false;
const areThereAnswersForQuestion = !CKEDITOR.instances.a_content;
Copy link
Member

@ScriptyChris ScriptyChris Jun 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@wizarddos wizarddos Jun 23, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

@ScriptyChris ScriptyChris Jun 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok.




document.querySelector('#q_doanswer').addEventListener('click', ()=>{
outdatedInfoContainerClassList.contains('hidden') ? outdatedInfoContainerClassList.remove('hidden') : outdatedInfoContainerClassList.add('hidden');
outdatedInfoContainerClassList.toggle('hidden');
Copy link
Member

@ScriptyChris ScriptyChris Jun 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@wizarddos wizarddos Jun 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

@ScriptyChris ScriptyChris Jun 23, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

if(areThereAnswersForQuestion){
const cancelAnswer = CKEDITOR.instances.a_content.element.$.form.docancel;
cancelAnswer.addEventListener('click', ()=>{
outdatedInfoContainerClassList.contains('hidden') ? outdatedInfoContainerClassList.remove('hidden') : outdatedInfoContainerClassList.add('hidden');
outdatedInfoContainerClassList.toggle('hidden');
Copy link
Member

@ScriptyChris ScriptyChris Jun 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}, {once: true});
}
})
Expand Down