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

Status Message - Improve UI #4842

Closed
mheppler opened this issue Jul 13, 2018 · 19 comments
Closed

Status Message - Improve UI #4842

mheppler opened this issue Jul 13, 2018 · 19 comments

Comments

@mheppler
Copy link
Contributor

While reviewing the new navbar language toggle contributed by community developer @JayanthyChengan, the design team determined that the :StatusMessageHeader label, which displays just to the right of the Dataverse branding, will need to be moved in order to improve it's ability to communicate messages, as well as clear out a lil more real estate for the new navbar link.

screen shot 2018-07-13 at 12 49 21 pm

One solution that we liked was similar to an example found in a Stack Overflow answer.

screen shot 2018-07-13 at 1 29 49 pm

@djbrooke
Copy link
Contributor

In backlog grooming today, there was some discussion about how it would be a little extra work to keep the dismissed message dismissed as the user navigates around the site.

@pdurbin
Copy link
Member

pdurbin commented Sep 28, 2018

I just made pull request #5118 and moved this issue to code review at https://waffle.io/IQSS/dataverse

@mheppler
Copy link
Contributor Author

mheppler commented Sep 28, 2018

Changed the UI component of the status header from a label to a warning alert. This will allow for the :StatusMessageHeader and :StatusMessageText to be displayed together.

There are some things still to determine for this new layout.

  • Do we need the popup anymore?
  • Should there be a limit to the amount of text you can add to the message? (We could truncate the message and display the entire message text in the popup. Screenshot is 260+ characters.)

Still have some minor HTML cleanup to do to get this ready to put back into code review, but wanted to get this screenshot in front of folks as well as the questions above.

mheppler added a commit that referenced this issue Oct 1, 2018
@scolapasta
Copy link
Contributor

With the entire status message now on screen, we want users to be able to dismiss this. If so, we need to store the fact that they dismissed in the session, so it doesn't just pop up when they navigate to another page. (unless there's some primefaces way to track this, but I don't know if they track that across the session automatically)

@mheppler
Copy link
Contributor Author

mheppler commented Oct 3, 2018

Removed previous screenshot of the layout in order to update it here based on changes approved by @TaniaSchlatter. Layout is good and checked in, just need to work on the cookies for the dismissible functionality.

screen shot 2018-10-03 at 2 02 18 pm

@matthew-a-dunlap
Copy link
Contributor

@mheppler I've wired up the dismiss button to a session based variable. Let me know what you think.

@mheppler
Copy link
Contributor Author

Looks good! The only question I had was if the status msg is X'd out while there no session, and then you log in, then there is no msg displayed. Should there be?

@matthew-a-dunlap
Copy link
Contributor

@mheppler I had the alert close following the flow of the other info in the session, so it gets cleared on logout but does not on login. I can add another hook to clear it on login as well if this doesn't seemed like enough for supporting multiple users sharing the same computer.

@mheppler mheppler removed their assignment Oct 15, 2018
@JayanthyChengan
Copy link
Contributor

@matthew-a-dunlap: I selected French language in the interface, but the status message still remains in English.

screen shot 2018-10-15 at 11 07 38 am

@JayanthyChengan
Copy link
Contributor

JayanthyChengan commented Oct 15, 2018

@matthew-a-dunlap:
Adding <f:view> in the harvestclients.xhtml solves the issue ( french interface displays the status message in french). Do we need to add <f:view in all the pages ?

<!DOCTYPE html>
<html xmlns="http://www.w3.org/1999/xhtml"
      xmlns:h="http://java.sun.com/jsf/html"
      xmlns:f="http://java.sun.com/jsf/core"
      xmlns:ui="http://java.sun.com/jsf/facelets"
      xmlns:jsf="http://xmlns.jcp.org/jsf"
      xmlns:p="http://primefaces.org/ui"
      xmlns:c="http://xmlns.jcp.org/jsp/jstl/core">
**<f:view locale="#{dataverseLocaleBean.localeCode}">**
    <h:head>
    </h:head>

    <h:body>

screen shot 2018-10-15 at 11 16 49 am

@juancorr
Copy link

juancorr commented Oct 15, 2018 via email

@juancorr
Copy link

Sorry , I have reply the e-mail and the screenshots are not shown

imagen


imagen


imagen

@JayanthyChengan
Copy link
Contributor

@juancorr: Thanks for the images,
In the third image - I see the status message in French when English language is selected. why is the status message not toggled to English

@juancorr
Copy link

Hi @JayanthyChengan,

I am not completely safe. I think that the status message is in Spanish because I have visited this page in Spanish and language is not updated in some places. I have not added the " <f:view>".

@juancorr
Copy link

I was wrong, this issue is not related to #5090. " <f:view>" tag translate the "Manage Harvesting Clients" but not the login page.

@scolapasta
Copy link
Contributor

FYI, those messages are not what is meant by the "status message" which is text defined in the settings. If there is an issue with these other messages, would you mind opening up a separate issue?

For the status message, the challenge for internationalization is that this is not text that is distributed with the code (or as an auxiliary component) but that is decided on by the admin. So an admin would need to decide to add an ad hoc status message and then all needed translations? That seems overwhelming. (from a technical perspective we could probably store each language status as a separate setting, or store the setting as a Map? Anyway, I suggest we keep the scope of this as primary language only and create a new issue where we can discuss how we will manage this for other languages.

@pdurbin
Copy link
Member

pdurbin commented Oct 16, 2018

@juancorr @JayanthyChengan to reiterate what @scolapasta is saying, can you please create a new issue for the bug you found? Thanks!

I'm moving this to QA after taking another look at pull request #5118 now that @mheppler and @matthew-a-dunlap have finished it off.

@JayanthyChengan
Copy link
Contributor

@pdurbin: created new issue #5202

@pdurbin
Copy link
Member

pdurbin commented Oct 16, 2018

@JayanthyChengan thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants