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

UI improvements #443

Merged
merged 9 commits into from
Jul 19, 2019
Merged

UI improvements #443

merged 9 commits into from
Jul 19, 2019

Conversation

mbollmann
Copy link
Member

This PR currently:

  1. Adds a button to reveal all co-authors and venues on author pages (and not just the top 5, though that is still the default view for reasons of clarity).
  2. Adds paper abstracts to list pages that are hidden by default, but can be toggled with a new "abs" button. Additionally, a button at the top of the page reveals all abstracts at once, which allows for using the browser's search function to search within all abstracts of a conference. Preview:

image

I would like some feedback on this, particularly on the following points:

  1. Adding abstracts to all list pages (i.e., volumes and events) increases their file size substantially. For example, the event page for ACL 2018 is already at 626K, but grows to 1.1M with abstracts. Does the added functionality warrant having such large HTML files?

  2. The button row before the paper title is getting quite crowded, especially with more and more additional material being accessible, e.g.:

    image

    Does anyone else feel this is a problem, or has an idea for how to make this nicer?

Marcel Bollmann added 3 commits July 1, 2019 20:47
To prevent visual clutter, only the top 5 are shown by default, but there is now
a button to show the full co-author/venue list.  Also reduced the padding
slightly to make the lists more compact.
@mbollmann mbollmann added the help wanted Interesting but beyond current volunteer bandwidth label Jul 2, 2019
Copy link
Member

@mjpost mjpost left a comment

Choose a reason for hiding this comment

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

This looks great to me. We should push it out.

hugo/layouts/events/single.html Show resolved Hide resolved
hugo/layouts/volumes/single.html Show resolved Hide resolved
@mjpost
Copy link
Member

mjpost commented Jul 11, 2019

I think a 1 MB event page isn't anything to worry about.

I agree the buttons are getting crowded. I'm not sure what to do. One idea is to reserve a fixed space to the right of the title, and put some less-essential buttons there, in a fixed format (so that one could scroll down and see easily which papers had a particular attachment type). But I don't think we need to solve that in this PR.

@mbollmann
Copy link
Member Author

What I'd particularly like is if the paper titles and author lists were lined up on the left, and didn't jump around depending on the number of attachments. :)

I'll refactor the ugly template code later and then give this a go.

@mjpost
Copy link
Member

mjpost commented Jul 11, 2019

Now that you mention it, that is bothersome :)

What about keeping (PDF, BIB, search) to the left, and putting all the others to the right of the title (right-justified)?

@mbollmann
Copy link
Member Author

What about keeping (PDF, BIB, search) to the left, and putting all the others to the right of the title (right-justified)?

I'm not quite convinced...

image

How useful/important do you think it is having the ACL IDs displayed in these lists? If we got rid of these here, maybe this would work:

image

@knmnyn
Copy link
Collaborator

knmnyn commented Jul 12, 2019 via email

@mjpost
Copy link
Member

mjpost commented Jul 12, 2019

Yes, you two are right. I really like your latest button mockup.

I do wonder if I'd miss having the Anthology IDs which I use often, but my use case (debugging changes to the Anthology!) is probably not very common. What about listing the IDs after the title in white text—this way, they could be searched for, but wouldn't be visible. e.g.,

Probabilistic FastText for Multi-Sense Word Embeddings [P18-1001]

(or maybe put the ID on the title line, but right justified, so one could also page through and easily track IDs?

This is looking very nice.

@mjpost
Copy link
Member

mjpost commented Jul 12, 2019

I really like how the title and author lines now line up vertically. If those are in the same <div>, maybe we could take care of #412 as well (which I agree would be useful) by (a) changing the author display to be "X, Y, and Z" and (b) adding a third line with something like the volume title and year? This would add some redundancy to the display, but I think the utility is high enough that it could be justified.

@mbollmann
Copy link
Member Author

I think (b) would add a lot of redundancy. It will be noticeable when you're scrolling through the full list of papers looking for something. But I'll keep #412 in mind when playing around with these changes.

Keeping the ACL ID around somewhere should also be possible. I'll play around with that too. If it's for local debugging purposes though, it's totally possible to set up Hugo so the IDs would appear locally (with a "development flag" set), but not in production... but I'm not sure that's what you had in mind.

I'll tune all these changes a bit before committing again, I want to make sure they look fine on all screen sizes first, but I should have something ready within the week. (So the ACL2019 crowd can profit from them. ;))

@mjpost
Copy link
Member

mjpost commented Jul 14, 2019

Yes, having this in place for ACL would be great. That will go live on the 28th, as you probably know.

You have convinced me that adding the conference citation to the volume page would be too much. Maybe we could just add a full citation at the top of the individual paper pages. Not sure if you want to do that in this commit or not.

@mjpost
Copy link
Member

mjpost commented Jul 15, 2019

Another thing to look out for is revisions which also crowd the display. Maybe we should just show this on the paper page and not the volume page.

0132E2CE-1498-412A-8B03-1A97632734E7

@akoehn
Copy link
Member

akoehn commented Jul 15, 2019

Versions: I don't know why they should be shown in the listing.

Paper ID: Never used the IDs on the listing, only the permanent URLs on the paper pages. Additional bonus: searching for the ID will only yield the paper site, not the listings.

Distinguishing between additional material (green buttons) and other seems like a good idea; I like the two rows concept!

Regarding the amount of buttons: Why does the anthology need a google search for the titles? The button is unintuitive (I would have guessed it does some internal search) and gives preference to a single search engine for no special reason. It is not even google scholar. It seems to me that we could live without that button.

Re #412: The listing seem to be the wrong place for that, I like the clear delimiters using |.

Page size

The pages are transferred as gzip anyway, so only about 1/8th of the size needs to be transferred. For the current ACL 2018 page, the html is 956kb, but only 126kb are transferred. (That is stil a surprising amount for the current site!)

The papers are linked with the full https://www.aclweb.org/ prefix even though that is not necessary. Would save us some bytes.

The hidden links with PDF extensions are on the event sites as well even though that is not needed. Changing that would introduce additional logic, right?

@mjpost
Copy link
Member

mjpost commented Jul 15, 2019

This has been clarifying. I second everything @akoehn just wrote.

@knmnyn
Copy link
Collaborator

knmnyn commented Jul 16, 2019 via email

@mbollmann
Copy link
Member Author

Thanks all, that has been very helpful!

Regarding the Search button: I agree that it can be removed in the paper listings, but there's also a (big) Search button the pager pages. It currently searches on Google, but I always thought searching on Google Scholar would make much more sense. That could be used, e.g., to quickly search for preprint versions or find citations. If you agree, I'd change the Search button on the paper pages to Google Scholar instead (and maybe make that clear in the icon/text).

The hidden links with PDF extensions are on the event sites as well even though that is not needed. Changing that would introduce additional logic, right?

Yes, and I'm actually not sure how that could be done. For Hugo, each row in these listings is just a "view" of the paper pages, and AFAIK that view knows nothing about the context in which it's rendered.

@akoehn
Copy link
Member

akoehn commented Jul 17, 2019

Maybe you could use the .Dir variable to distinguish between the two types? Should be possible if one can do something like .Dir startswith "/people" (I have never written hugo templates so I don't know)

Agree with google scholar.

@mbollmann
Copy link
Member Author

Maybe you could use the .Dir variable to distinguish between the two types? Should be possible if one can do something like .Dir startswith "/people" (I have never written hugo templates so I don't know)

Unfortunately not, the context is that of the page (= paper) being rendered, so .Dir will always be papers/.../.../.

The only thing we could do is move the invisible PDF link into its own template function, and call that separately in the templates where it should appear. It would appear in a different position then compared to now, but that's hopefully irrelevant?

@akoehn
Copy link
Member

akoehn commented Jul 17, 2019

@mbollmann if it is not straight-forward, I would just postpone this issue. The change will be invisible to users, so no need to do this as part of this pull request.

@mbollmann
Copy link
Member Author

Okay, I think this should be ready now.

Before:
image

After:
image

The button part has a fixed minimum width to make everything line up nicely, and will expand (and break the alignment) only if a paper has more than four attachments, which doesn't appear to be the case for any paper yet. Slight downside is that older volumes without abstracts/attachments have a lot more whitespace now:
image

Some other changes:

  • Added Academicons to change search icon to Google Scholar logo. (Best way I could come up with to indicate that the button leads to GS without adding large amounts of text.)
  • Removed the ACL IDs on all list entries now, and used the --environment variable of Hugo to reintroduce them when it is set to development. Since that's also Hugo's default, I changed the Makefile to explicitly set it to production.

@mbollmann mbollmann marked this pull request as ready for review July 18, 2019 14:21
@mbollmann mbollmann requested a review from mjpost July 18, 2019 14:58
Copy link
Member

@mjpost mjpost left a comment

Choose a reason for hiding this comment

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

This looks very nice!

@mjpost
Copy link
Member

mjpost commented Jul 18, 2019

I've built it and like the look. One comment is that the width spacing for front matter seems different:

image

I'll merge later today, in case anyone else wants to comment.

@akoehn
Copy link
Member

akoehn commented Jul 18, 2019

@mjpost: I'm currently building it and will have a look as well.

Maybe let @mbollmann merge it, then he can decide whether he wants to do a squash- or a merge-commit.

@mjpost
Copy link
Member

mjpost commented Jul 18, 2019

Sounds good. I'll leave it to you two to review further and merge.

@akoehn akoehn mentioned this pull request Jul 18, 2019
@akoehn
Copy link
Member

akoehn commented Jul 18, 2019

@mjpost Everything is well-aligned on my system, I can't reproduce your problem regarding spacing.

Everything looks good to me, go ahead and merge!

A small point which is not a blocker and could be dealt with separately as well (but it is a UI improvement):
The large "you are viewing the latest version ..." banner is not needed anymore in my opinion. The version shown is by definition the latest one and we haven't received substantial feedback in #170 in the last months. Instead, the link at the bottom ("issues") should be changed to "report issues". Otherwise, I would have assumed it was a link to some errata page or similar. Getting rid of the banner saves quite some space.

@mjpost
Copy link
Member

mjpost commented Jul 18, 2019

I agree that the banner seems dated at this point. It is however I think useful to have an easy way to report feedback. We've had several people click on that button and add some feedback to that long thread, and I suspect at least some of them may not have gone through the trouble of opening a new issue. Though it's hard to know. I do support removing it if that's the consensus.

Which reminds me, it'd be nice to have an easy way to update the MOTD box without having to commit and rebuild...

@mbollmann
Copy link
Member Author

I've built it and like the look. One comment is that the width spacing for front matter seems different:

Seems that the badges are just a tiny bit wider on your system/browser. Maybe I can increase the minimum width just a tiny bit to compensate.

I agree that the banner seems dated at this point. It is however I think useful to have an easy way to report feedback.

I would suggest moving the feedback button to the sidebar on the landing page. That way it's still at least somewhat prominent there.

@mbollmann
Copy link
Member Author

Merging this now, since I'll be offline without a laptop until Tuesday. I'm happy to push some fine-tuning changes (and do sth about the feedback banner) when I return.

@mbollmann mbollmann merged commit 665e22f into master Jul 19, 2019
najtin pushed a commit to ir-anthology/ir-anthology that referenced this pull request Jun 9, 2021
UI improvements:

+ Allow to see all co-authors and venues on people pages
+ Add abstracts to paper list entries
+ Change layout of buttons in paper list entries
+ Remove ACL IDs everywhere (except on paper detail pages)
+ Change search buttons to Google Scholar everywhere
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Interesting but beyond current volunteer bandwidth
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants