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

Add icon for VMware Photon OS #484

Merged
merged 3 commits into from
Oct 20, 2022
Merged

Add icon for VMware Photon OS #484

merged 3 commits into from
Oct 20, 2022

Conversation

erwindon
Copy link
Owner

Photon OS ships with Salt 3005.1, and it would be nice if there was a supported icon for it. I tried creating one called os-vmware-photon-os.png in the images directory, but it doesn't seem to load. Not being a js guy, I may be misunderstanding the logic in Panels.js to load the icon.

lsb_distrib_description	≡	"VMware Photon OS 4.0"
lsb_distrib_id	≡	"VMware Photon OS"
os	≡	"VMware Photon OS"
os_family	≡	"RedHat"
osarch	≡	"x86_64"
oscodename	≡	"Photon"
osfinger	≡	"VMware Photon OS-4"
osfullname	≡	"VMware Photon OS"
osmajorrelease	≡	4
osrelease	≡	"4.0"
osrelease_info	≡	[
    4,
    0
]

@erwindon
Copy link
Owner

changed to PR

@erwindon
Copy link
Owner

erwindon commented Oct 20, 2022

it was actually a bug!
for convenience, I do not use files that have blanks/spaces inside them.
in the translation for the OS identification to a filename, we replace spaces with '-'.
but the code only translated the first one!
I've updated to code to replace all of them

I found an image that I think is the correct one and added it too.

afbeelding
(the version number "11" is from my "Debian 11" installation)

the change is on the git branch from this PR
can you please try it?

@erwindon erwindon requested a review from mkbrown October 20, 2022 09:40
@sonarcloud
Copy link

sonarcloud bot commented Oct 20, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@erwindon erwindon removed the request for review from mkbrown October 20, 2022 09:56
@erwindon erwindon removed their assignment Oct 20, 2022
@mkbrown
Copy link
Author

mkbrown commented Oct 20, 2022

@erwindon , I downloaded a full copy of this branch, and replaced my saltgui interface with it (just getting started with salt, so no big deal). Still having issues where it's not properly replacing the image name. 404 errors in the js console

Failed to load resource: the server responded with a status of 404 (Not Found)![](http://vger:3333/static/images/os-vmware-photon%20os.png)

Tried replacing the regex with the replaceAll function, no joy either. Same result.

const pngName = pImageName.replaceAll(' ', '-').toLowerCase() + ".png";

It's always the things that look like they should be simple, but turn out not to be... ;-p

Thanks for the help with this!

@erwindon
Copy link
Owner

this happens a lot and I do not always warn for it:
your browser may have cached an older version.
please use REFRESH of the page in your browser.
you may even need shift-REFRESH.

@erwindon
Copy link
Owner

as a side note:
We cannot use replaceAll in SaltGUI because that function was added only roughly 2 years ago in JS.
see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/replaceAll and https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/replace
some users have browsers that are older than 2 years...

@mkbrown
Copy link
Author

mkbrown commented Oct 20, 2022

Reverted the replaceAll, and forced a refresh of the browser (and checked with another browser as well). It's working perfectly now.

I'll admit to the DOH! moment... didn't occur to me that caching might be the issue (server guy).

Thanks for the assistance and the quick fix! Good to merge at your convenience!

@mkbrown mkbrown removed their assignment Oct 20, 2022
@erwindon erwindon merged commit 270f6d5 into master Oct 20, 2022
@erwindon erwindon deleted the photon-os branch October 20, 2022 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants