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

Added image path for icons when toggle to dark theme #6356

Merged

Conversation

Darshan-upadhyay1110
Copy link
Contributor

Change-Id: I14a99df7cf10bf73f898f15d3aaa5a32c9cd51af

Current case when we toggle to dark mode icons are not getting change because we are using default image path when we call get image url.

so i changed it according to the theme we are using and if icon does not present in dark theme then use the default icon.

@pedropintosilva , @eszkadev => need your review for this changes i made

browser/src/core/LOUtil.js Fixed Show resolved Hide resolved
@Darshan-upadhyay1110 Darshan-upadhyay1110 force-pushed the dark-theme-icon branch 2 times, most recently from fa76619 to 1f3b6df Compare May 15, 2023 12:07
@eszkadev
Copy link
Contributor

looks good :)

can we do one more improvement:
merge L.LOUtil.getImageURL and L.LOUtil.checkIfImageExists into one function ?
something similar to:

L.LOUtil.setImage(img, name, doctype) {
    img.src = L.LOUtil.getImageURL(name, doctype);
    L.LOUtil.checkIfImageExists(img)
}

then we have to use one function instead 2 :)

@Darshan-upadhyay1110 Darshan-upadhyay1110 force-pushed the dark-theme-icon branch 2 times, most recently from 4b02cf1 to b1ae392 Compare May 16, 2023 12:44
Copy link
Contributor

@eszkadev eszkadev left a comment

Choose a reason for hiding this comment

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

works as expected :)

@Darshan-upadhyay1110 Darshan-upadhyay1110 force-pushed the dark-theme-icon branch 2 times, most recently from 11e005b to 413e4d5 Compare May 19, 2023 13:27
@Darshan-upadhyay1110 Darshan-upadhyay1110 force-pushed the dark-theme-icon branch 2 times, most recently from e174ea4 to 99ab0ca Compare May 22, 2023 06:50
@Darshan-upadhyay1110 Darshan-upadhyay1110 force-pushed the dark-theme-icon branch 2 times, most recently from 3c60c19 to 69470e5 Compare May 24, 2023 12:31
@Darshan-upadhyay1110 Darshan-upadhyay1110 force-pushed the dark-theme-icon branch 2 times, most recently from 9655771 to d93156a Compare May 24, 2023 12:55
Signed-off-by: Darshan-upadhyay1110 <darshan.upadhyay@collabora.com>
Change-Id: I14a99df7cf10bf73f898f15d3aaa5a32c9cd51af
Copy link
Contributor

@eszkadev eszkadev left a comment

Choose a reason for hiding this comment

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

looks good, @pedropintosilva if you are ok we can merge

@pedropintosilva
Copy link
Contributor

Thanks, pulling now to test

Copy link
Contributor

@pedropintosilva pedropintosilva left a comment

Choose a reason for hiding this comment

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

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

branding icon folder
3 participants