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

Fixes #2461: Build Log dashboard should use same layout/CSS as rest of Dashboard #2480

Merged
merged 3 commits into from
Nov 20, 2021

Conversation

mqnguyen5
Copy link
Contributor

@mqnguyen5 mqnguyen5 commented Nov 17, 2021

Issue This PR Addresses

Type of Change

  • UI: Change which improves UI

Description

This issue updates the Build Log dashboard to have the same layout/CSS as the rest of the Telescope dashboard.

Changes:

  • Added header and sidebar
  • Replaced Material UI logo with Telescope's logo and changed sidebar title to "Telescope Dashboard"
  • Updated the Telescope logo inside the sidebar to redirect to ./

Results:

Screen Shot 2021-11-17 at 12 04 51 AM

Screen Shot 2021-11-17 at 12 05 17 AM

Checklist

  • Quality: This PR builds and passes our npm test and works locally
  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not (if applicable)
  • Documentation: This PR includes updated/added documentation to user exposed functionality or configuration variables are added/changed or an explanation of why it does not(if applicable)

@gitpod-io
Copy link

gitpod-io bot commented Nov 17, 2021

@mqnguyen5
Copy link
Contributor Author

Currently, this PR is not fully completed, as I still have some stuff to discuss in #2472.

@humphd
Copy link
Contributor

humphd commented Nov 18, 2021

Currently, this PR is not fully completed, as I still have some stuff to discuss in #2472.

What's the status of this? Also, please post a screenshot of it running.

@mqnguyen5
Copy link
Contributor Author

@humphd I'll push the rest of the changes up by today.

@mqnguyen5
Copy link
Contributor Author

mqnguyen5 commented Nov 18, 2021

I have made some changes based on what was discussed in #2472:

Screenshots:

  • Main page

Screen Shot 2021-11-18 at 4 02 35 PM

  • Sidenav

Screen Shot 2021-11-18 at 4 22 52 PM

	* added header and sidenav
	* replaced Material UI logo in sidenav to Telescope's logo
	* removed spare double-quotes in svg link and scrollbar in sidenav
	* updated link in sidenav logo to redirect to ./
	* added link to Telescope override css files
	* removed unused elements in top nav
	* updated links and titles in sidenav
@humphd
Copy link
Contributor

humphd commented Nov 18, 2021

@mqnguyen5 can you do a single screenshot of the whole thing, which is really want we want to see here?

@mqnguyen5
Copy link
Contributor Author

mqnguyen5 commented Nov 18, 2021

Here it is:

Screen Shot 2021-11-18 at 6 11 49 PM

I assume it means a screenshot of the whole page with the sidenav opened?

@humphd
Copy link
Contributor

humphd commented Nov 18, 2021

Let's add a left margin to the terminal so it has some breathing room next to the sidebar. Also, let's have it go higher in the page (don't waste so much space above it). I think it can be aligned with the top of the sidebar. Let's also have it fill the available space vertically.

@mqnguyen5
Copy link
Contributor Author

mqnguyen5 commented Nov 18, 2021

If that's the case, should I remove the "Build Log" <h1> above the terminal? And the left margin should be added only when we open the sidenav?

@humphd
Copy link
Contributor

humphd commented Nov 18, 2021

I think so, since the sidebar acts as the anchoring navigational element (we don't need a title).

@mqnguyen5
Copy link
Contributor Author

Currently, the page looks like this after remove the title and adjust the padding to make it closer to the top:

Screen Shot 2021-11-18 at 6 44 42 PM

I'm not sure how I can add a margin when opening the sidenav and remove it when closing the sidenav. Is there a way to do it with Material UI?

@mqnguyen5
Copy link
Contributor Author

@Andrewnt219, @menghif you have any suggestions on this?

@mqnguyen5 mqnguyen5 requested a review from menghif November 19, 2021 18:25
@Andrewnt219
Copy link
Contributor

Andrewnt219 commented Nov 19, 2021

@mqnguyen5 one quick way is you copy everything in the index.html, paste it on the build.html. Open Devtools, start deleting all the unrelated stuffs (with Element Inspector), copy the html from the Devtools and paste it back to your IDE. Finally, just add the tag div.id="termminal" to main content area.

@Andrewnt219
Copy link
Contributor

I'm not sure how I can add a margin when opening the sidenav and remove it when closing the sidenav. Is there a way to do it with Material UI?

That sounds like JS magic. But why do you want to do this?

This is just Bootstrap 5 with vanilla JS btw, not the same as Material UI on Telescope.

@mqnguyen5
Copy link
Contributor Author

After looking around, I managed to find out what is wrong. I didn't not realized that I had been showing screenshots of the site when opening in small window instead of going full screen.

Screen Shot 2021-11-19 at 4 01 10 PM

I also adjusted the padding on the terminal per @humphd suggestion, so it should take less space from the top now.

@drew5494 drew5494 self-requested a review November 19, 2021 23:30
Copy link
Contributor

@drew5494 drew5494 left a comment

Choose a reason for hiding this comment

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

Tested and looks good on my end.

@humphd humphd merged commit 235baa3 into Seneca-CDOT:master Nov 20, 2021
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