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

feature(messages): improved UI for messages listing #7343

Merged
merged 1 commit into from
Oct 22, 2014

Conversation

jeabakker
Copy link
Member

The checkbox for bulk actions was moved to the beginning of the row and
an excerpt of the message is shown in the listing

solves: #4739

TODO:

  • Update commit message footer to be Fixes #4739
  • Add screenshots from before/after the changes
  • Describe testing strategy for changes

@ewinslow
Copy link
Contributor

Sounds like a nice improvement! Can you post screenshots of before/after and describe what you did to test the changes?

The checkbox for bulk actions was moved to the beginning of the row and
an excerpt of the message is shown in the listing

fixes: Elgg#4739
@jeabakker
Copy link
Member Author

Before:
7343-messages_before
After:
7343-messages_after

Testing:

  • tried the toggle button
  • checked one, clicked 'mark read'

@ewinslow
Copy link
Contributor

Sweet. Definitely seems like an improvement.

Won't block on these, since it was so bad before, but some ideas:

  • remove the delete button at the right of each message. Since we have the checkboxes and delete button at the bottom I think that's enough. Delete should be a pretty rare action anyways.
  • vertically center the checkboxes and avatars.

@ewinslow
Copy link
Contributor

Another suggestion: use a <table> for rendering this. Would be more semantic than the image blocks strategy we currently use. Only issue there is themes.

@hypeJunction
Copy link
Contributor

No tables in core views, please. Makes life hell styling them for
responsive themes. Instead, I suggest we improve the elgg grid. Increase it
to 12, if not 24 columns.
On Oct 21, 2014 6:39 PM, "Evan Winslow" notifications@github.com wrote:

Another suggestion: use a

for rendering this. Would be more
semantic than the image blocks strategy we currently use. Only issue there
is themes.


Reply to this email directly or view it on GitHub
#7343 (comment).

@ewinslow
Copy link
Contributor

Disagree. This is a table. It should use table. If you want it more flexible you should use the viewtype system.

@hypeJunction
Copy link
Contributor

I will not make noise if it's done after #4449 is fixed. Otherwise I am strongly opposed to tables and will use every chance to remind you

@hypeJunction
Copy link
Contributor

Additionally, table layout would be a breaking change, and will break every project I have worked on.

@ewinslow
Copy link
Contributor

The breaking change argument is more compelling.

On Tue, Oct 21, 2014, 3:03 PM Ismayil Khayredinov notifications@github.com
wrote:

Additionally, table layout would be a breaking change, and will break
every project I have worked on.


Reply to this email directly or view it on GitHub
#7343 (comment).

@PerJensen
Copy link
Contributor

We could use CSS tables here, they are easier to turn responsive than actual tables. Will still require pseudo-elements though, which probably is bad for IE.

It would also make sence with table header, name, subject, etc.

@ewinslow
Copy link
Contributor

OK, in the interest of not stalling this out, I'm merging. We can continue the table discussion elsewhere.

ewinslow added a commit that referenced this pull request Oct 22, 2014
feature(messages): improved UI for messages listing
@ewinslow ewinslow merged commit a18d310 into Elgg:1.x Oct 22, 2014
@jeabakker jeabakker deleted the messages-checkbox-layout branch October 13, 2016 11:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants