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

11.0 mig web listview invert selection #1004

Merged
merged 4 commits into from
Mar 5, 2019

Conversation

MeetKD
Copy link

@MeetKD MeetKD commented Jul 29, 2018

No description provided.

espo-tony and others added 3 commits July 29, 2018 20:51
* [10.0][ADD] Module web_listview_invert_selection

* [FIX] Multiple code reviews

* [FIX] Changed single-quotes to double-quotes for improving codacy tests;
@MeetKD MeetKD mentioned this pull request Jul 29, 2018
68 tasks
@pedrobaeza pedrobaeza added this to the 11.0 milestone Jul 30, 2018
Copy link

@espo-tony espo-tony left a comment

Choose a reason for hiding this comment

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

Hi,
I tested this module and it works perfectly. My only concern is for the look of the "Invert selection" button, which I find a bit out of place with the rest of a listview layout. Do you think is it possible to give to the button a look which is more flat so that it can better merge with the rest of the page?
Thank you

@MeetKD
Copy link
Author

MeetKD commented Aug 4, 2018

@espo-tony Agree with you. 👍

Copy link
Member

@tarteo tarteo left a comment

Choose a reason for hiding this comment

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

Only some annoying nitpicking. 😳

@@ -0,0 +1,64 @@
.. image:: https://img.shields.io/badge/licence-AGPL--3-blue.svg
Copy link
Member

Choose a reason for hiding this comment

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

Not important but it would be nice if you use the new partitioned readme.

'aria-label': _t('Invert Selection'), 'title': _t('Invert Selection')});
return $('<' + tag + '>')
.addClass('o_invert_selection')
.append($icon);
Copy link
Member

Choose a reason for hiding this comment

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

Also not blocking but I think using $.wrap() here is a bit cleaner.

@@ -0,0 +1,5 @@
.o_list_view {
.o_invert_selection {
padding-left: 2px;
Copy link
Member

Choose a reason for hiding this comment

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

Can you remove the padding? I think it looks better without it.

@tarteo
Copy link
Member

tarteo commented Dec 18, 2018

@MeetKD Are you planning to finish this?

Copy link
Member

@tarteo tarteo left a comment

Choose a reason for hiding this comment

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

My comments were non-blocking

@tarteo tarteo merged commit c509fa8 into OCA:11.0 Mar 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants