Scorrimento alla categoria corrente interrotto a partire da 2.8.0.beta5

Bug
1

Nelle versioni di Discourse precedenti alla 2.8.0.beta5, se modifichi un argomento e fai clic sul menu a discesa Categoria, la categoria corrente viene sia selezionata che fatta scorrere nell’elenco delle categorie, in modo che sia immediatamente visibile in alto e le sue sottocategorie (se presenti) siano visibili. L’argomento può quindi essere facilmente spostato in una sottocategoria più appropriata.

Funzionamento in 2.8.0.beta4 e versioni precedenti:

A partire dalla 2.8.0.beta5, la categoria corrente non viene più fatta scorrere quando si apre il menu a discesa Categoria:

Spero che queste brevi dimostrazioni video siano utili per mostrare rapidamente ciò che sto cercando di descrivere.

Nota: nei miei esempi ho le opzioni “fixed category positions” e “fixed category positions on create” attivate, tuttavia il comportamento di scorrimento alla posizione corrente è mancante con queste opzioni attivate o disattivate.

Ho confermato questo in uno sviluppo locale installando la 2.8.0.beta4 e osservando il comportamento desiderato, e poi aggiornando alla 2.8.0.beta5 e osservando che il comportamento non funzionava più.

Ho esaminato il codice finché non ho trovato che il seguente grande commit che coinvolge select-kit.js è dove è avvenuta la modifica (aspettate un momento e dovrebbe saltare alle righe di codice in questione):

Il seguente codice è stato rimosso dal metodo _scrollToRow(), che viene chiamato dal metodo _scrollToCurrent():

if (rowContainer) {
  const collectionContainer = rowContainer.parentNode;

  collectionContainer.scrollTop =
    rowContainer.offsetTop - collectionContainer.offsetTop;
}

Ho confermato che riaggiungere questo codice a select-kit.js nella 2.8.0.beta5 risolve il problema, anche se non mi è chiaro perché sia stato rimosso, sono incerto su altri possibili effetti collaterali dell’aggiungerlo di nuovo.

Grazie per aver esaminato questo problema, e spero che possiamo far funzionare di nuovo questo comportamento in una futura versione.

4 Mi Piace
3

Ciao, grazie per questo :+1:

Non ricordo esattamente perché l’ho fatto, ma per quanto ne so ora, penso che anche questo sia un errore.

Ho creato questa PR:

Se puoi provare questo codice e dirmi se pensi che ti basterebbe, per favore?

4

Ciao Joffrey, grazie per aver dato un’occhiata a questo! Ho testato le ultime modifiche, tuttavia sembra che non abbiano funzionato. Poiché è stato unito, l’ho confermato sul branch tests-passed.

Dei due commit nel PR, il primo funziona (ab0fbf1), ma viene sovrascritto dal secondo (92943ff).

La modifica alla riga 992 sembra a posto ma non ha l’effetto desiderato. Sembra che abbia bisogno del blocco di codice che è stato rimosso e che regola il valore di collectionContainer.scrollTop.

Grazie ancora!

1 Mi Piace
5

Sì, non ero sicuro che questo comportamento sarebbe stato sufficiente. Posso aggiungerlo di nuovo domani. Puoi essere più preciso su cosa esattamente non ti piace di questo comportamento ora, per favore?

6

Ciao Joffrey,

Grazie per continuare a indagare su questo. Con le attuali modifiche al codice (mi sono assicurato di scaricare il codice più recente, cancellare tmp e riavviare il server di sviluppo), non ha recuperato il comportamento presente nelle versioni precedenti alla 2.8.0.beta5. Ancora non scorre per mostrare la categoria corrente tra le opzioni visibili nel menu a discesa.

Il comportamento corretto/desiderato è quello mostrato nel primo video nel post originale, tuttavia attualmente funziona ancora come nel secondo video. Attualmente la barra di scorrimento inizia sempre completamente in alto, indipendentemente da dove si trovi la categoria corrente tra le opzioni.

La riga rowContainer?.focus({ preventScroll }); sembra non avere alcun effetto in questo caso, indipendentemente dal fatto che preventScroll sia true, false, o anche se l’intera riga viene rimossa.

La riga essenziale che fa funzionare il comportamento (non presente nella versione attuale) è:

collectionContainer.scrollTop =
          rowContainer.offsetTop - collectionContainer.offsetTop;

Senza questa, non scorre nella posizione corretta, poiché scrollTop è 0 per impostazione predefinita. Dopo l’esecuzione di questa riga, scrollTop equivale al numero corretto di pixel che deve scorrere verso il basso.

Spero che questo sia utile. Per favore, fammi sapere se posso aiutare a chiarire ulteriormente.

Grazie!

7

Sì, non è in cima ma è nell’area di visualizzazione, che avrei detto fosse sufficiente.

8

Ciao Joffrey,

Grazie per il feedback. Ho aggiunto più categorie alla mia demo e ho confermato che la categoria corrente ora appare nella viewport, decisamente un miglioramento. Sarebbe comunque ideale che apparisse in cima all’elenco, come era solita fare. Ciò garantirebbe che più sottocategorie sottostanti siano immediatamente visibili e l’utente possa fare clic per passare a una sottocategoria senza prima scorrere verso il basso per rivelarle. A mio parere, questo è un caso d’uso più forte rispetto alla visualizzazione di categorie adiacenti non correlate. È stato modificato per essere centrato di proposito o è stata una regressione accidentale del comportamento precedente? In ogni caso, a tua discrezione, offro solo la mia opinione.

Grazie ancora e apprezzo il tuo duro lavoro su Discourse!