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

Dark Theme with Matrix Update #374

Conversation

aguero-tech
Copy link

Dark Theme + Matrix Page updates. refactored code to replace var with const and let. To allow for matrix select all and clear all buttons along with Matrix table update respectively

image

@wurstbrot wurstbrot requested review from vbakke and 0x41head March 21, 2025 06:08
@vbakke
Copy link
Collaborator

vbakke commented Mar 23, 2025

Thank you for your contribution @aguero-tech. I like the dark mode.

However, I'd prefer the pull request to be purely dark mode and, and not having Subdimension filter buttons mixed in. The filters are a different topic, and should be treated separately.

When it comes to the dark mode, it looks like it's not quite finished. When I go to the circular heatmap under the Overview menu, the heatmap is still quite white, and the details of an activity is not converted.
image
Same goes for the Teams page.

The details under the Matrix look good though. The current green color code goes well with the suggested dark background, which is a benefit.

The toggle button Switch to Light/Dark Mode needs to fit more into the current UI design.

I don't see any obvious place for it with the current setup. @wurstbrot, it has been mentioned being able to change team names of local browser (i.e. for the official dsomm.owasp.org site). Maybe we should have a Settings menu where we control light/dark theme, and the team names?

@aguero-tech, I like the work you have done. If this is your first contribution, I guess you were just not aware of all the facets to DSOMM. (I too was surprised by the actual complexity to the site. : ) It is more than what first meets the eye.

@0x41head: Do you have any further comments?

@aguero-tech
Copy link
Author

aguero-tech commented Mar 23, 2025

Hi @vbakke – thanks so much for the thoughtful feedback!

You’re absolutely right — this is my first contribution, and I definitely started to feel things getting a bit more involved than expected. That’s why I also submitted a separate PR focused solely on the Dark Theme updates (excluding the Matrix changes): #373.

That said, I’ll be incorporating all of your feedback related to color contrast — including the DSOMM title, Overview, Teams page, and the Matrix graph — to ensure everything displays clearly in dark mode. I really appreciate the time you took to review everything, and your professionalism means a lot!

I’ll keep an eye on both this PR and #373 for any additional comments while I work on cleaning up those details. Thanks again!

@vbakke
Copy link
Collaborator

vbakke commented Mar 23, 2025

Your welcome, @aguero-tech . Yeah, I was a bit confused as to which PR and branch to look at. This PR had the most recent commit, so I chose aguero/forcontribution-matrixupdate.

The circular overview is SVG generated. The shade of green is calculated based on the fraction of "completed" activities, and is based on white, and grey for N/A. If you are not familiar with SVG let me know, and we find a way when the rest is completed. We should also have to look at the black colour in the dependency graph. But that also need a better overhaul in my opinion.

(Filters: The current filters have been bugging me as well. Just leave it out of this PR. I'll post a new Issue shortly. I recommend that we follow the practice of BI (business intelligence) tools. Where no selection is treated as selected all filters. Then any first click will focus on that one filter, and not like now, removing that filter from focus.)

@vbakke
Copy link
Collaborator

vbakke commented Mar 23, 2025

@aguero-tech, just FYI: @yashchauhan4579 has just submitted #379, an upgrade of Angular, and they plan to continue to upgrade until Angualr 19 is reached.

This may (or may not) affect your changes. So, please make sure you merge the latest dsomm/main branch into your branch before resubmitting this PR. (Depending on how quickly each of you will progress, of course, and when PRs get approved : )

@aguero-tech aguero-tech mentioned this pull request Mar 25, 2025
@aguero-tech
Copy link
Author

postponing matrix effort until after angular is done migrating. @vbakke please follow up with #381 on the Dark Theme topic. I have incorporated your improvements. Thank you!

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.

2 participants