-
Notifications
You must be signed in to change notification settings - Fork 24
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
Minor layout redesign + prereg button #98
Conversation
393b546
to
8cc9add
Compare
The menu items aren't yet functional, but this makes the UI more compact.
Move term switcher to be next to schedule switcher; move "maintained to SIPB" to be next to the logo
It doesn't need to be a button anymore now that it's in the header.
some thoughts on the design:
|
While the "Maintained by SIPB" looks a little out of place by the logo, it looks better to me than having it below the "import" section where it was previously. Right now, it looks a bit dystopian beneath the logo, but it does a good job of indicating development by SIPB without actually being in the way. I do think the import section (similar to what CJ said) looks very cluttered now, and the format should be changed to look cleaner or have the text be shorter. |
I do kind of agree — it feels a little corporate. Do you have any better ideas?
Yeah… I agree. I tried shortening the text for these buttons but couldn't think of any satisfying ways of shortening them. I think a dropdown might be too discreet, though; people might not realize that these options exist.
Hmm, interesting. I can try that. |
Fixes #97.
|
as an alternative to the button, you could have icon-only buttons with tooltips??? |
Gonna give me two cents really quick...
|
i dont know if this is a layout or a style issue? but i just put it in this branch since this is where we're working on improving the ui
The "rename" button doesn't consistently autofill with the current schedule name, though.
I feel like the theme button shouldn't be next to the schedule selector; it controls the appearance of all of Hydrant rather than being local to a particular schedule or term. |
I think this is ready for review? I added some things and made some changes based on feedback from people in the SIPB office :) |
Looks good to me (though I can't approve my own PR). I'm happy to merge if you approve, @dtemkin1 (unless anyone has other thoughts). |
@@ -3,6 +3,7 @@ | |||
<head> | |||
<title>Hydrant</title> | |||
<meta charset="UTF-8" /> | |||
<link rel="icon" type="image/svg+xml" href="/hydrant.svg" /> |
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.
</DialogHeader> | ||
<DialogBody> | ||
<Flex direction="column" gap={4}> | ||
<Text> |
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.
I just copy-pasted this from what was previously in privacy.html, but most of it isn't really applicable since we don't currently have GCal integration. Still, good to leave in for whenever we fix it I suppose
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.
Looks good to me! I left some comments on things we can think about for the future, but don't necessarily have to be fixed before merging.
@tangluna suggested yesterday that we move some things around on Hydrant, so I whipped up this proof of concept and also folded #97 into it.
Before:
After:
I moved the term selector to be near the schedule selector (which is more tightly coupled functionality-wise) rather than the logo and "change theme" button, and moved the "maintained by SIPB" to accompany the Hydrant logo; the schedule buttons are also moved behind a menu now.
Known issues: