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

New design for sort options #2161

Closed
trang opened this issue Feb 23, 2020 · 21 comments · Fixed by #2400
Closed

New design for sort options #2161

trang opened this issue Feb 23, 2020 · 21 comments · Fixed by #2400
Assignees
Labels
enhancement Issue that describes a problem that requires a change in the current functionalities of Tatoeba.
Milestone

Comments

@trang
Copy link
Member

trang commented Feb 23, 2020

We discussed in #2108 about a new design for the sort options.

The idea looks basically like this:
Screenshot_2020-02-01 All public lists (2) - Tatoeba(1)

See #2108 (comment) and the next few replies for the wording of each option.

Consider implementing this together with #1063.

@trang trang added the enhancement Issue that describes a problem that requires a change in the current functionalities of Tatoeba. label Feb 23, 2020
@gioele-santi
Copy link
Contributor

So instead of the line with 4 anchors we should add a button with a drop down list with all possible sort options.
I would like to work on this.

@trang
Copy link
Member Author

trang commented Jun 7, 2020

Great, I am assigning this to you then! 😄

Please make sure to check the comment mentioned in the description, as there is some draft of code already.

There are several places where the "Sort" links are used, so it is not always 4 anchors. But yes, the point is to remove those links and replace them with a button + dropdown list instead.

The pages where the "Sort" links are being used are listed in another comment in the same thread.

I suggest you implement this new design in one only page at first, then do a draft PR like you did previously. We can check if it's alright on this page and you can finish with the other pages once we have settled down on the wording.

@gioele-santi
Copy link
Contributor

Yesterday I finally had time to start studying this issue.Since there are several pages with the same inline sort options, I decided to focus on one first, in particular src/Template/SentenceLists/index.ctp.
First I focus on the styling.

The box has alway a top bar with <div class="md-toolbar-tools"> with a title inside.
To add a button I am using <md-button class="md-icon-button" aria-label="Sort by">

I am checking if there's an existing css class to allign on right, otherwise I found out that I can get the right placement with this kind of styling:

position: absolute;
margin-right: 1em !important;
right: 0px;

My main concern with the looks is how to load an icon in md-icon

<md-icon md-svg-src="img/list.svg"> 

tries to load http://localhost:8080/eng/sentences_lists/img/list.svg instead of http://localhost:8080/img/list.svg (which should be the right path (then I will find the proper sort icon)

Then I will figure out how to display the dropdown list (I have to understand ho to display it, maybe using javascript and an a select)

@trang
Copy link
Member Author

trang commented Jun 16, 2020

I am checking if there's an existing css class to allign on right

Rather than aligning to the right, you can make the <h2> element expand to take all the available space. That is done by adding the flex attribute in the element (<h2 flex>).

My main concern with the looks is how to load an icon in md-icon

At the moment we are not using SVGs but the Material Font Icons with ligatures.

<md-icon>sort</md-icon>

Then I will figure out how to display the dropdown list

Perhaps you've missed the info but there is already an example of how to display the sort menu. I should have probably posted here all the relevant info from the other thread.

Code sample

        <md-toolbar class="md-hue-2">
            <div class="md-toolbar-tools">
                <h2 flex><?= $title ?></h2>

                <md-menu md-offset="5 50" md-position-mode="target-right target">
                    <md-button ng-click="$mdOpenMenu($event)">
                        <md-icon>sort</md-icon> Sort by
                    </md-button>
                    <md-menu-content>
                        <md-menu-item ng-repeat="item in ['list', 2, 22223, 9324, 02934, 12]">
                            <md-button>
                                <md-icon>{{ $index === 1 ? 'check' : 'blank'}}</md-icon>
                                <span style="padding-right: 12px">Option {{item}}</span>
                            </md-button>
                        </md-menu-item>
                    </md-menu-content>
                </md-menu>
            </div>
        </md-toolbar>

If you replace the toolbar code in src/Template/SentencesLists/index.ctp by this code, you should see exactly what is displayed in the screenshot.

Sort option labels

For each sort link that we currently have, there will be 2 options in the menu. Below is the list of every page where there are sort links and the corresponding suggested labels.

https://tatoeba.org/eng/sentences_lists/index

  • list name
    • List name (alphabetical)
    • List name (reverse alphabetical)
  • date created
    • Newest first
    • Oldest
  • number of sentences
    • Highest number of sentences
    • Lowest number of sentences
  • last updated
    • Most recently updated
    • Least recently updated

https://tatoeba.org/eng/sentences_lists/show/9633

  • date added to list
    • Most recently added
    • Least recently added
  • date created
    • Newest sentences
    • Oldest sentences

https://tatoeba.org/eng/tags/view_all

  • count
    • Highest number of sentences
    • Lowest number of sentences
  • name
    • Tag name (alphabetical)
    • Tag name (reverse alphabetical)

https://tatoeba.org/eng/tags/show_sentences_with_tag/2139

  • date created
    • Newest sentences first
    • Oldest sentences first
  • date of tag
    • Most recently tagged
    • Least recently tagged

https://tatoeba.org/eng/users/all

  • Username
    • Username (alphabetical)
    • Username (reverse alphabetical)
  • Member since
    • Newest first
    • Oldest first
  • Member status
    • Status (admin to contributor)
    • Status (contributor to admin)

https://tatoeba.org/eng/sentences/of_user/TRANG

  • date modified
    • Most recently updated
    • Least recently updated
  • date created
    • Newest first
    • Oldest first

https://tatoeba.org/eng/reviews/of/TRANG

  • date modified
    • Most recently updated
    • Least recently updated
  • date created
    • Newest first
    • Oldest first
  • sentence id
    • Newest sentences
    • Oldest sentences

There was also some mention of the sort section from the advanced search but we can leave this out of scope for this issue.

@gioele-santi
Copy link
Contributor

@trang Thank you very much. I checked and it works as expected. I am sorry but I didn't think about checking about the other discussion.
Now I am studying how the buttons should work.
As I understand in current version links are generated using CakePHP Paginator.
In the new menu the list of buttons is generated in ng-repeat = "item in [...]".

I was wondering, should I put links inside the menu or try to work with a directive in app.module.js ?
So that when clicking I open the page with the right sort option via js
or create code in background using php (maybe Paginator)?

@trang
Copy link
Member Author

trang commented Jun 25, 2020

No problem @gioele-santi 🙂

In the code sample, the ng-repeat was used only to quickly generate menu items. In your case, you won't have much use of ng-repeat because there's no angular data that holds the information for the options. You have to define manually the HTML for each menu item.

You can replace the <md-button> by link that is generated by $this->Paginator->sort() and it should still display properly.

<md-menu-item>
    <?= $this->Paginator->sort( ... ) ?>
</md-menu-item>

I'll leave it to you to figure out how to handle the indicator for the current selected sort.

@gioele-santi
Copy link
Contributor

Thank you very much.
Next days I will be able to finally take care of this.
I thought I had more time during summer but unfortunately a very big deadline came up with university and since I am the only developer there now it is taking a lot of my efforts (by the way the experience with the other issue helped me improve the javascript approach in the university project).

@gioele-santi
Copy link
Contributor

Ok, today I was able to reprise my work on this issue.
I fixed aspect of drop down menu and its alignment in the top bar.

Currently options are visualized with this code:

<md-menu-item>
    <md-button>
        <md-icon>{{ $index === 1 ? 'check' : 'blank'}}</md-icon>
        <span style="padding-right: 12px">
            <?php echo $this->Paginator->sort('created', __('date created')); ?>
        </span>
   </md-button>
</md-menu-item>

I already know the management of md-icon is not (yet) correct.
I think I need to better figure out how Paginator->sort() works to visualize the options.

First parameter should be the database column used for sorting, second parameter the localized name for the link.

Ascending or descending has no direct control, so I am not sure if I can generate two links for each options as ascending/descending are automatically alternated by Paginator->sort().
So I should not double the links in the drop down list right? (eg: date ascending/date descending)

Then I am focusing on the check mark with md-icon as of now the only way to figure out if an option was selected is to check the <a></a> element to see if it has class asc or desc.
Should I hack that or is there any other way to figure out which option is selected?

@AndiPersti
Copy link
Contributor

Ascending or descending has no direct control, so I am not sure if I can generate two links for each options as ascending/descending are automatically alternated by Paginator->sort().
So I should not double the links in the drop down list right? (eg: date ascending/date descending)

You should create two links for each sort key, see Trang's comment above.
Paginator->sort() takes as a third parameter an options array which makes it possible to create links with a locked direction:

echo $this->Paginator->sort('name', __('list name'), ['direction' => 'asc', 'lock' => true]);
// Result: <a class="desc locked" href="/eng/sentences_lists/index?sort=name&amp;direction=asc">list name</a>
echo $this->Paginator->sort('name', __('list name'), ['direction' => 'desc', 'lock' => true]);
// Result: <a class="asc locked" href="/eng/sentences_lists/index?sort=name&amp;direction=desc">list name</a>

Then I am focusing on the check mark with md-icon as of now the only way to figure out if an option was selected is to check the <a></a> element to see if it has class asc or desc.
Should I hack that or is there any other way to figure out which option is selected?

Since we reload the page after each new sort selection, you can set the correct icon already on the server (i.e. in the PHP code). Paginator->sortKey() and Paginator->sorDir() give you the current sort key and direction respectively.

@gioele-santi
Copy link
Contributor

Thank you. I looked for documentation about Paginator but it is hard to find.
I would also like to see the documentation to better understand it.
It should be in CakePHP documentation, right? Can you point me the page so I can also read the description there?
I was looking at this but it lacks examples for sort.

@AndiPersti
Copy link
Contributor

You want to look at the documentation for the PaginatorHelper.
CakePHP's API reference is also quite helpful.

@gioele-santi
Copy link
Contributor

Thank you very much!

@gioele-santi
Copy link
Contributor

Ok I have updated the first page with new sort options menu. My last doubt is about translations, is there a list of translation keys I can use for the double options?

Meanwhile I will write the code in other pages.

@trang
Copy link
Member Author

trang commented Jul 21, 2020

My last doubt is about translations, is there a list of translation keys I can use for the double options?

I'm not sure to understand what you mean exactly but if you are talking about translations of the UI strings, the "translation key" is the string itself. You have to encapsulate strings in the function __() to make them translatable. You can check the README in /src/Locale for more details on how we localize strings.

@gioele-santi
Copy link
Contributor

I mean: in the list above there are some suggested labels. Should I use them even if there's not yet a translation? (I am not sure, I should check).

About sorting I need to check what's the exact parameter name for user status (users/all)

Then the only page where I have some troubles with sentence_lists/show/ (the second one in the list above), as the structure is a little different and it looks like most of html is actually generated by php code.

@gioele-santi
Copy link
Contributor

By the way, I remember you once pointed me to a page with a list of standard users (ids and passwords) for testing the local website. I cannot find it anymore, could you show me a link again as I want to test the new sort lists with some content...

@trang
Copy link
Member Author

trang commented Jul 22, 2020

I mean: in the list above there are some suggested labels. Should I use them even if there's not yet a translation?

Yes, you should use these labels even if there's no translation yet. We will get them translated once your pull request is merged.

About sorting I need to check what's the exact parameter name for user status (users/all)

The param is role. The sort link is defined in the sortForRole() function in the PaginationHelper.

You can also see it in the URL if you click on the "Member status" link to change the sort direction.

Then the only page where I have some troubles with sentence_lists/show/

Right, that page actually has two version (for old design and new design). You should implement the change only for the new design. The corresponding template file is called show_angular.ctp.

By the way, I remember you once pointed me to a page with a list of standard users (ids and passwords) for testing the local website.

This information can be found on the README of the imouto repository.

→ You can log in using one of these accounts: admin, corpus_maintainer, advanced_contributor, contributor, inactive and spammer. For all of them the password is 123456.

@gioele-santi
Copy link
Contributor

Thank you. I already started changing the labels.

@gioele-santi
Copy link
Contributor

Ok I created some sentences, tags and reviews and tested my changes (I looked many times at that README page but I don't know why I always skipped the line with password, probably I expected to find it at the end of page...).

Anyway most of the pages work as expected but sentences_lists/show/# and sentences/of_user/... does not seem to work as expected. They still look the same as before. I think I may have modified some other file. Show should be src/Template/SentencesLists/show_angular.ctp as you told me before while the other one should be src/Template/SentencesLists/of_user.ctp.

Am I right? Is there any configuration or difference in the downloaded project that may influence this or did I change the wrong files?

@trang
Copy link
Member Author

trang commented Jul 24, 2020

For /sentences_lists/show/ you will need to make sure that in the user Settings, the option "Display sentences with the new design..." is enabled.

For /sentences/of_user, you edited the wrong template (or you edited the correct template but looked at the wrong page 🙂).

  • /sentences/of_usersrc/Template/Sentences/of.user.ctp
  • /sentences_lists/of_usersrc/Template/SentencesLists/of_user.ctp

@gioele-santi
Copy link
Contributor

For /sentences_lists/show/ you will need to make sure that in the user Settings, the option "Display sentences with the new design..." is enabled.

Luckily today I had no problem with that, maybe I changed the option yesterday while I was looking for review option.

For /sentences/of_user, you edited the wrong template (or you edited the correct template but looked at the wrong page 🙂).

  • /sentences/of_usersrc/Template/Sentences/of.user.ctp
  • /sentences_lists/of_usersrc/Template/SentencesLists/of_user.ctp

I see. In fact I edited sentences_lists of user instead of sentences. It is not in the description above but since it had the sort links I think I should leave it with the updates, I just changed it to use the same sort options used in sentences list.

@trang trang linked a pull request Aug 4, 2020 that will close this issue
@trang trang added this to the 2020-08-03 milestone Aug 4, 2020
@trang trang closed this as completed Aug 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issue that describes a problem that requires a change in the current functionalities of Tatoeba.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants