Skip to content

Conversation

@Alex-Jordan
Copy link
Contributor

This is one last (I hope) change I'd like to propose to sidebar navigation. With this, if you are acting as some user, then you have a direct link in the sidebar nav to see their user detail page (which has more details about all the sets assigned to them, including test versions).

The only current way (that I know of) to navigate here is to go to the Classlist Editor and click the link in the "Assigned Sets" column. That still works with this. But also now if you arrive at that page this way, you will see this is where you are in the sidebar nav.

If you are acting as user X and you go to Classlist Editor and click the link in the "Assigned Sets" column for user Y, you will see two links in the sidebar (assuming X ≠ Y). The one for Y is active, and the one for X is just visible because you are acting as X. Here is a screenshot with all of that going on, consistent with Statistics and Student Progress.

Screen Shot 2023-06-09 at 1 59 43 PM

Copy link
Member

@drgrice1 drgrice1 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 good to me. Although, there is one thing I am not certain of. We haven't been adding the dir="ltr" attribute for user ids in the site nav. Notice that the student progress and statistics sub links do not have that. @taniwallach: Should we be doing that?

@drgrice1
Copy link
Member

Thinking about it more, I suspect that we should have the dir="ltr" attribute on all of the student id links in the site nav. This is because at this point we don't allow utf8 characters like those in the Hebrew alphabet in user ids. However, it is probably best to let @taniwallach confirm this.

@Alex-Jordan
Copy link
Contributor Author

I added dir="ltr" to the other user ID links in the site nav.

There are many other places where a user ID or username is printed to screen. Consider this screenshot after I switched to Hebrew:

Screen Shot 2023-06-12 at 11 20 22 AM

No inline directions are specified here, but the browser is doing the right thing. Reading form the right, there is Hebrew, then a correct switch to ltr for the user names, then back to rtl for the period. The browser is probably seeing that this run of characters are from a code block that typically uses ltr, and makes that change.

Actually if I explicitly set the sidebar nav links to rtl, I still see them presented by the browser left-to-right.

This is only with one browser of course, and I don't know how all this plays out with a screen reader.

@pstaabp
Copy link
Member

pstaabp commented Jun 12, 2023

I like this, but wonder about the number of instructor tools now available/visible on the nav menu. Are there too many? Perhaps we can visually break them up with horizontal line(s). I don't have a good location though.

@Alex-Jordan
Copy link
Contributor Author

Note that most of the time, this particular change does not make the nav menu any longer. It only does so when you are acting as another user, or if you do something like click the link under the Assigned Sets column in the Classlist Editor.

@taniwallach
Copy link
Member

About the dir="ltr": Unicode bidi typically works reasonable well if there is no mixture of content with multiple directions. The behavior of arrow keys in input boxes can get very confusing, and things like paths+filename with slashes and periods are not easy to handle when in RTL mode.

I would say that anywhere you expect "latin only" text with spaces and punctuation (course_id, student_id, filenames) putting on dir="ltr" is probably helpful for Hebrew and Arabic sites. Eventually, when I finally manage to free time to install WW 2.18 RC on a server, I'll try to collect places where adding it can help.

@pstaabp
Copy link
Member

pstaabp commented Jun 15, 2023

Note that most of the time, this particular change does not make the nav menu any longer. It only does so when you are acting as another user, or if you do something like click the link under the Assigned Sets column in the Classlist Editor.

True.

Copy link
Member

@pstaabp pstaabp left a comment

Choose a reason for hiding this comment

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

I would say to merge and if @taniwallach runs across this or other places that we need to update the text direction that we have another PR for that.

@pstaabp pstaabp merged commit 9aa0435 into openwebwork:WeBWorK-2.18 Jun 15, 2023
@Alex-Jordan Alex-Jordan deleted the userDetail branch August 18, 2024 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants