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.

Sign up for GitHub

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

Jump to bottom

Theme changes #712

Merged
merged 9 commits into from
Jun 28, 2023
Merged

Theme changes #712

merged 9 commits into from
Jun 28, 2023

Conversation

iamhks
Copy link
Contributor

@iamhks iamhks commented May 29, 2023 •
edited
Loading

Summary

Closes #343

Changes

  • Added an api for the backend
  • Made a call to that api from frontend to fetch theme data
  • Refactored all the hard-coded values to use the colors from fetched theme data

Screenshots

This is the backend:
Screenshot 2023-06-04 at 22 57 07

This is the theme page of backend:
Screenshot 2023-06-07 at 02 11 24

This is how the UI looks when I set everything to green in index.css:
Screenshot 2023-06-04 at 21 55 01

This is the UI back to normal when I fetch the default colors from the backend.
Screenshot 2023-06-04 at 21 55 16

Requests / Responses

Request
(Only for backend)
GET /api/theme Returns the latest theme saved by admin

Response
(Only for backend)
HTTP/1.1 200 OK

{
    "id": 7,
    "Theme_Name": "Default",
    "Primary_Color1": "#DC3545",
    "Primary_Color2": "#FDCB00",
    "Primary_Color3": "#00B8C4",
    "Secondary_Color1": "#FFCDD2",
    "Secondary_Color2": "#A94442",
    "Secondary_Color3": "#FFF7D4",
    "Secondary_Color4": "#9F861E",
    "Secondary_Color5": "#E0F6F4",
    "Secondary_Color6": "#03848C",
    "Text_Color1": "#212121",
    "Text_Color2": "#757474",
    "Text_Color3": "#E4E4E4"
}

@tuxology tuxology requested a review from kamthamc June 4, 2023 17:13
@tuxology
Copy link
Member

tuxology commented Jun 4, 2023

@kamthamc Hey, @iamhks is working on this still. If you have some early comments on his direction, please let him know. Hemant and I have been discussing some ideas here as well: https://unstructured.zulipchat.com/#narrow/stream/246561-ZubHub/topic/Accepted.20projects/near/362547771

@iamhks iamhks marked this pull request as ready for review June 4, 2023 17:30
@iamhks
Copy link
Contributor Author

iamhks commented Jun 4, 2023

Hi @tuxology and @kamthamc. I think I am good for the first review of this PR. I have attached the screenshots and api details in the description. Please have a look and provide your feedback.

@iamhks iamhks changed the base branch from master to themeChanges June 4, 2023 17:44
kamthamc
kamthamc reviewed Jun 13, 2023
View reviewed changes
Copy link
Collaborator

@kamthamc kamthamc left a comment

Choose a reason for hiding this comment

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

👏🏻 Nice work. I think some of the color / backgrounds are left out.

..._frontend/zubhub/src/assets/js/styles/components/projects_drafts/projectsDraftsGridStyles.js
@@ -19,18 +19,18 @@ const styles = theme => ({

tabStyle: {
'&.Mui-selected': {
color: '#00B8C4',
color: 'var(--primary-color3)',
fontWeight: 'bold',
backgroundColor: '#ffffff',
Copy link
Collaborator

Choose a reason for hiding this comment

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

background color might need to use css variable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is color white, which we haven't asked input from the backend

Copy link

Choose a reason for hiding this comment

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

Sure but if the user sets the background color as a light color its hard to read, we might have use the text colora

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't touch the code where we used plain white, and the fact that we are not allowing to update the background color here, this issue would not rise I guess?

...ub_frontend/zubhub/src/assets/js/styles/components/user_activitylog/userActivitylogStyles.js
@@ -31,7 +31,7 @@ const styles = themes => ({
paddingLeft: '1.5%',
paddingRight: '20px',
'&:hover': {
backgroundColor: '#FFF7D4',
backgroundColor: 'var(--secondary-color3)',
fontColor: '#FFFFFF',
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this supposed to be color or fontcolor?

zubhub_frontend/zubhub/src/assets/js/styles/views/faqs/FAQsStyles.js
background:
'linear-gradient(to bottom, rgba(255,204,0,1) 0%, rgba(255,229,133,1) 25%, rgba(255,255,255,1) 61%, rgba(255,255,255,1) 100%)',
'linear-gradient(to bottom, var(--primary-color2) 0%, var(--primary-color2) 25%, rgba(255,255,255,1) 61%, rgba(255,255,255,1) 100%)',
Copy link
Collaborator

Choose a reason for hiding this comment

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

some colors are missing css variables

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is white color which we haven't taken any input from in the backend

zubhub_frontend/zubhub/src/assets/js/styles/views/group_members/groupMembersStyles.js
filter:
"progid:DXImageTransform.Microsoft.gradient( startColorstr='#ffcc00', endColorstr='#ffffff', GradientType=0 )",
"progid:DXImageTransform.Microsoft.gradient( startColorstr='var(--primary-color2)', endColorstr='#ffffff', GradientType=0 )",
Copy link
Collaborator

Choose a reason for hiding this comment

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

missing css variable

@iamhks iamhks merged commit eb10c43 into unstructuredstudio:themeChanges Jun 28, 2023
@tuxology tuxology mentioned this pull request Jul 9, 2023
Merged
@tuxology tuxology mentioned this pull request Aug 4, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kamthamc kamthamc kamthamc left review comments

@chaitanya-cloudknox chaitanya-cloudknox chaitanya-cloudknox left review comments

@NdibeRaymond NdibeRaymond Awaiting requested review from NdibeRaymond NdibeRaymond is a code owner

@tuxology tuxology Awaiting requested review from tuxology tuxology is a code owner

@ajain1921 ajain1921 Awaiting requested review from ajain1921

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Allow basic theme changes to be done from Django Admin
4 participants
@iamhks @tuxology @kamthamc @chaitanya-cloudknox