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

Directory view enhancement #28

Merged
merged 5 commits into from
Feb 21, 2018

Conversation

proxyconcept
Copy link

Following #25

sort icons should be more close than their column name to avoid sorting on bad column name

Done (adjustment in css)

no tel: or mail: links for each row (in comparison with quick/advanced search results)

Done (using same code already made for search results)

link on each row to display entry : will be nice to have a clickable row rather than an icon. Not so easy because we need to keep tel: and mail: links. Maybe a link on firstname/lastname.

Done (see new config option $directory_linkto)

generalize display/pagination to quick/advanced search with rows (as @coudot said)

Todo (not related to this part, see #1)

number of elements display on pages :
add a PHP option to set default number (currently = 10)

Done (see new config option $default_page_length)

number of elements display on pages :
add additional values in dropdown : "all" and/or an input field to enter a number (for example : 250)

Done (with value -1 for All)

print preview is not usuable

Done, i think, the problem reported is unclear...
I see that with the print css, all URL (href) are displayed and it's not very convenient. This came with default bootstrap CSS. I have added a css definition to remove this fonctionality.

Copy link
Member

@coudot coudot left a comment

Choose a reason for hiding this comment

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

Agree with all changes.

@coudot coudot merged commit 4f6efac into ltb-project:master Feb 21, 2018
@coudot
Copy link
Member

coudot commented Feb 21, 2018

@nqb we now have to create a new documentation page about this feature

@nqb
Copy link
Contributor

nqb commented Feb 22, 2018

@coudot : I will do it

@nqb
Copy link
Contributor

nqb commented Feb 22, 2018

@proxyconcept, @coudot : to my mind, "Directory" should be translate in "Annuaire" rather than "Répertoire".

Your opinion ?

@nqb
Copy link
Contributor

nqb commented Feb 22, 2018

My comments on this last PR :

sort icons should be more close than their column name to avoid sorting on bad column name

Good for me except on IE 11.

no tel: or mail: links for each row (in comparison with quick/advanced search results)

Links ok. I notice difference on empty fields (in comparison with quick/advanced search results) :

  • Directory view : empty fields appears with
  • Quick/Advanced search view : empty fields appears with a blank

link on each row to display entry : will be nice to have a clickable row rather than an icon. Not so easy because we need to keep tel: and mail: links. Maybe a link on firstname/lastname.

It works but too much links, it's a bit confuse.

Is it possible to have a whole row clickable without that links and visual change on line (to see its clickable) ? By visual change, I mean color change or text change. I don't know best practices for that kind of Web UI.

number of elements display on pages :
add a PHP option to set default number (currently = 10)

If I set up $default_page_length = 250; in config file, I have an unexpected result : 10 value selected in dropdown but 250 results by page.

I wonder if $default_page_length should be $directory_default_page_length. Not sure if we use this var in future forquick/advanced search results.

print preview is not usuable

It works as expected. However Showing _START_ to _END_ of _TOTAL_ entries (filtered from _MAX_ total entries)" is not well displayed in print preview when using Firefox with more than 5 pages :
print_preview

@coudot
Copy link
Member

coudot commented Feb 22, 2018

Is it possible to have a whole row clickable without that links and visual change on line (to see its clickable) ? By visual change, I mean color change or text change. I don't know best practices for that kind of Web UI.

Seems possible with CSS (tr:hover) and some js to intercept the click, but there will still be the problem of email and tel links.

It works as expected. However Showing START to END of TOTAL entries (filtered from MAX total entries)" is not well displayed in print preview when using Firefox with more than 5 pages

Maybe we can add CSS to hide these information in print view.

@proxyconcept
Copy link
Author

Good for me except on IE 11

Hmm sorry I can't test easily with Windows browser.

I notice difference on empty fields (in comparison with quick/advanced search results)...

For me, the next step is to share the same view between the directory list and the search results in rows. I have a work in progress for a new PR soon...

It works but too much links, it's a bit confuse.

I add a specific css style for mailt: and tel: links (see proxyconcept@b5c534b )

Is it possible to have a whole row clickable...

I don't know if it's a good idea, it's a matter of choice so it should be based on the configuration.
I already add the $directory_linkto with an array of clickable columns. I think reuse the same variable, accepting a boolean value at true if you want the whole line clickable.

If I set up $default_page_length = 250; in config file...

Wrong value : 250 is not in the list. It's why I add this comment "// 10, 25, 50, 100 or -1 for all"
I don't set a config variable for the list of length values... I don't know if it's usefull or if a list of hard-coded values is enough for everyone.

Not sure if we use this var in future forquick/advanced search results.

Why not ? As I say before, I think it's a good idea know to share code between directory listing and search results (in rows mode). We could use the same templates, with differents variables if necessary (columns list for example). But it seem useless for me to have different default page lengths.

However "Showing START to END of TOTAL entries"...

Should be fix with #31 : Print CSS hide Datatables navigation blocks.

@coudot
Copy link
Member

coudot commented Feb 23, 2018

@proxyconcept yes, we need to use same template between rows mode and directory view.

@nqb
Copy link
Contributor

nqb commented Feb 27, 2018

Hmm sorry I can't test easily with Windows browser.

See my screenshot
ie11

Two improvements that can be done with Datatables :

  • Browsing buttons at the top of pages. Currently, only at the end of pages.
  • Possibility to go on specific page. Currently, if you have 50 pages and you want to go on page 12, it's really painful.

@coudot and @proxyconcept, you didn't answer to my question

to my mind, "Directory" should be translate in "Annuaire" rather than "Répertoire".

Your opinion ?

@proxyconcept
Copy link
Author

Two improvements that can be done with Datatables ...

I don't think it's a good idea to add suggestions on this entry. The PR is merged... this thread should be close and new demands would be better in new issues.

to my mind, "Directory" should be translate in "Annuaire" rather than "Répertoire"

Je me permet le français pour cette réponse ! De base je n'ai aucune opinion... Ensuite si je regarde des définitions :

  • répertoire : Inventaire (liste, recueil…) où les matières sont classées dans un ordre qui permet de les retrouver facilement.
  • annuaire : Recueil publié annuellement et qui contient des renseignements remis à jour tous les ans.

Du coup "répertoire" me semble bien être le terme le plus approprié.

@coudot
Copy link
Member

coudot commented Feb 28, 2018

I don't think it's a good idea to add suggestions on this entry. The PR is merged... this thread should be close and new demands would be better in new issues.

Indeed, we should have a new PR or issue on this. We may close it only if we all agree on the result.

Du coup "répertoire" me semble bien être le terme le plus approprié.

Moi également, parler d'annuaire ici peut prêter à confusion, sachant que l'annuaire est aussi le serveur contenant toutes les données.

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.

3 participants