-
Notifications
You must be signed in to change notification settings - Fork 29
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
Vertical content cards #2470
Vertical content cards #2470
Conversation
…e right of title, list view toggle made into button group and moved to right
…DoenetTools into verticalContentCards
get and set preferred folder view
…DoenetTools into verticalContentCards
Something broke with tile view so that the tiles are no longer centered horizontally when there is just one or two of them. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When there is no content, the Activities
and the Assigned
look quite bleak in list view with everything white and no visual distinction between the header and rest of the body. Maybe make it another color, like it is in card view, to give the page a little more texture.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's an interesting UI question for which I don't have a good answer. If one clicks an editable title and then clicks away to remove the focus from the title, should that click navigate to an activity if the click lands on an activity card? It does for both card view and list view.
For card view, I left it with the current behavior as I didn't know how to change it, but I'm not sure if it is the best choice. Especially in list view where one has the whole screen filled with activities, I'm worried there wouldn't be much "neutral" (non-link) screen space to click on to remove focus from the title without navigating to an activity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In Activities, could you add a column with "Assignment status" or something like that shows if an assignment is open or closed (maybe blank if not assigned?). Also, in "Assigned" could you add an "Assignment status" or something like that header above that information?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For responsive design, we want the screens to be able to go down to 344 pixels width (at least that is the smallest I get when selecting different phones in Chrome's web developer tools). For the list view, you'll want to compress the spacing for narrower screens. (See Chakra's Responsive Styles for info on how to do that.) For the narrower screens, we don't want any left-to-right scrolling. If you need to wrap text or stack items vertically, that's better than having horizontal scrolling.
…ntent, fixed assignment status column in activities, made it so that for card and list mode first click after blurring title textbox does nothing
…DoenetTools into verticalContentCards
I think you're right about being able to click away from a textbox without triggering the click event on another row/card. I think I found a solution that works for the cards and table, and it feels a lot more natural that way, being able to click anywhere. I tested it and it seems to work as expected for both views (the click event is disabled one time and not any more after that), but let me know if you notice any weirdness with it. |
I like it better this new way. I'm not sure what people expect. I guess we'll see how as people use it. The only worry is that the mouse cursor changes to a pointer when moving over a button, so I wonder if that will confuse people if it doesn't actually click the button on the first try. We'll have to test this out in usability studies at some point. |
OK, I found some strange behavior. Click to edit a title on a folder/activity. Then select part of the title with your mouse, move your mouse pointer outside the title input so that it is still on the item and let go of the mouse pointer. It navigates to that item! I don't know if we have to worry about this, but it definitely is strange! |
…andles the case of dragging the text onto a link
Wow good catch! I had to get my brother to help replicate it, because I couldn't figure it out. I just pushed something that I think handles that case and hopefully doesn't break anything from before! I added a whole new state to address it, and I'm not sure if that's worth it, but I think it does fix it. |
Great. Let me know if you want to add the responsive design components to this PR. Or, I can merge it in as is, and you can work on responsive design with another PR. I already deployed this to dev2.doenet.org so that other can check it out. |
Cool! It's up to you; I started it, but I'm not sure I'm going about it right and it could take me another couple of days. If you want to get this merged in soon, I don't mind making another PR. |
… title in Activities page
client/src/Widgets/ActivityTable.tsx
Outdated
<Th></Th> | ||
{showPublicStatus ? <Th>Visibility</Th> : null} | ||
<Show above="md"> | ||
{" "} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apparently, having such white space " " (here and on the next line) isn't allowed inside a <tr>
. It's triggering the warning (visible in the Javascript console):
Warning: validateDOMNesting(...):
Whitespace text nodes cannot appear as a child of<tr>
. Make sure you don't have any extra whitespace between tags on each line of your source code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm I can't remembering adding that or figure out why it's needed. I think I'm just going to get rid of it, unless you know of any reason why we need it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, we don't need it. It was probably something added by prettier at one point. I've noticed those appearing sometimes when formatting the document, but I'm not sure what causes them.
|
||
function saveUpdatedTitle(newTitle: string) { | ||
if (newTitle !== activity.title && activity.id !== undefined) { | ||
fetcher.submit( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't let the new title be a blank "" or you won't be able to change it back here, due to the <Editable>
being too small. Instead, if newTitle===""
, then just set the name to "Untitled".
Fix regression in changing content name from Doenet#2472
In order to make things consistent, do you think it makes more sense to switch everything to be an Editable, as in ActivityTable, or an Input/Text combo like in ContentCard? |
I don't have a strong feeling, but you'd think there would be some benefits of Chakra's Editable. I'm not sure what. It's possible it's better designed for accessibility. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving and merging this PR to make applying the next PRs easier. We can resolve remaining issues in another PR.
This PR adds as a list view of the content of a folder.