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

webui: adding PRETTY_NAME to use in title instead of anaconda generic title #4830

Merged
merged 1 commit into from
Jun 19, 2023

Conversation

acruzgon
Copy link
Contributor

According to UX, "Anaconda" should never show to people to avoid confusion. This PR is to start using PRETTY_NAME instead of the generic Anaconda title.

Before:
image

After:
image

@acruzgon acruzgon added the webui label Jun 12, 2023
Copy link
Contributor

@KKoukiou KKoukiou left a comment

Choose a reason for hiding this comment

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

Some small fixups needed. Also tests :)

ui/webui/src/components/app.jsx Outdated Show resolved Hide resolved
ui/webui/src/components/app.jsx Outdated Show resolved Hide resolved
ui/webui/src/helpers/conf.js Outdated Show resolved Hide resolved
ui/webui/src/helpers/conf.js Outdated Show resolved Hide resolved
Copy link

@garrett garrett left a comment

Choose a reason for hiding this comment

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

Thanks for making this change!

There's a minor difference what what I intended: I was thinking of having the full distribution name + version at the top and the welcome to message only have the distribution name (without version).

Copy link
Contributor

@VladimirSlavik VladimirSlavik left a comment

Choose a reason for hiding this comment

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

Thank you!

  1. I'm not sure this is the right place where the data should come from. Take a look at pyanaconda/product.py - that loads /.buildstamp and environment variables. This should work similarly, in fact we already get isFinal from buildstamp in betanag.js. Sorry, I see this is proposed to be done like this. In that case it's fine.

  2. I don't think conf.js is the right place for the helper. That file contains the custom parser for anaconda config, which is a pretty specific thing. Maybe some other helper with things for the "product" information? Maybe some reorganization is in order, so that the betanag stuff and anything else reading buildstamp are together?

Apart from that, this looks good to me.

@acruzgon
Copy link
Contributor Author

acruzgon commented Jun 16, 2023

@garrett it would like this:
image
is that okay? also do you want to keep ! after the welcome message?

ui/webui/src/components/app.jsx Outdated Show resolved Hide resolved
@acruzgon acruzgon force-pushed the INSTALLER-2953 branch 2 times, most recently from 894d3f4 to 88504b5 Compare June 16, 2023 14:17
@acruzgon acruzgon requested a review from KKoukiou June 16, 2023 14:17
@acruzgon acruzgon force-pushed the INSTALLER-2953 branch 2 times, most recently from 5cb9ea9 to ec8b796 Compare June 16, 2023 15:14
Copy link
Contributor

@KKoukiou KKoukiou left a comment

Choose a reason for hiding this comment

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

Please don't include merge commits and/or empty commits in your PRs.

@KKoukiou KKoukiou force-pushed the INSTALLER-2953 branch 2 times, most recently from 58d660d to 61020e6 Compare June 19, 2023 08:02
@KKoukiou
Copy link
Contributor

/kickstart-test --waive webui only

@KKoukiou KKoukiou force-pushed the INSTALLER-2953 branch 3 times, most recently from 392e9ee to ae39a5b Compare June 19, 2023 09:02
@garrett
Copy link

garrett commented Jun 19, 2023

is that okay?

@acruzgon: That looks good!

also do you want to keep ! after the welcome message?

No, we don't need to. It would fit right in with Fedora, but would probably be a bit much in the context of RHEL and CentOS.

@KKoukiou
Copy link
Contributor

/kickstart-test --waive webui only

@KKoukiou KKoukiou merged commit decb65a into rhinstaller:master Jun 19, 2023
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants