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

Remove the timezone map from the Time & Date spoke #5404

Merged
merged 3 commits into from
Jan 22, 2024

Conversation

poncovka
Copy link
Contributor

@poncovka poncovka commented Jan 15, 2024

The timezone map was removed from the Time & Date spoke in the GKT-based user interface
and the spoke was redesigned to accommodate the changes. The installer no longer depends
on the the libtimezonemap package.

TODO:

  • Build and test a boot.iso.
  • Run kickstart tests for time.
  • Test the live media.

See: #5355

Remove the timezone map, so we can drop the dependency on the libtimezonemap
package. This package won't be available on some systems in the future.
Update the format of this file to Glade 3.40.
@pep8speaks
Copy link

pep8speaks commented Jan 15, 2024

Hello @poncovka! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 54:1: E402 module level import not at top of file

Comment last updated at 2024-01-16 14:59:18 UTC

@poncovka
Copy link
Contributor Author

Redesign with NTP enabled:
timezone_redesign_ntp

Redesign with manual time enabled:
timezone_redesign_manual

@mairin
Copy link
Contributor

mairin commented Jan 15, 2024

Redesign with NTP enabled: timezone_redesign_ntp

Redesign with manual time enabled: timezone_redesign_manual

This is looking great @poncovka !! The only thing I would suggest is maybe 3x the padding between the Time Zone region block and the Radio buttons, and between the radio buttons I'd probably do 2x the padding. I believe GTK padding according to the guidelines of this version of GTK was factors of 6. So maybe 18px padding between the two major blocks, and 12px padding between the radio button options.

@poncovka
Copy link
Contributor Author

poncovka commented Jan 15, 2024

So maybe 18px padding between the two major blocks, and 12px padding between the radio button options.

Like this?
timezone_redesign_ntp_18_12

Or the padding should be additional to the current spacing?
timezone_redesign_ntp_30_24

@mairin
Copy link
Contributor

mairin commented Jan 15, 2024

@poncovka The 2nd one with more padding - that's great!

@mairin
Copy link
Contributor

mairin commented Jan 15, 2024

Ah while we're here, sorry, one quick tweak if not a pain - a little more padding between the date dropdowns and the "/" characters inbetween the widgets... maybe just 6 px?

@poncovka
Copy link
Contributor Author

Ah while we're here, sorry, one quick tweak if not a pain - a little more padding between the date dropdowns and the "/" characters inbetween the widgets... maybe just 6 px?

Like this? I tried it with two different locales (en_US and cs_CZ):
timezone_us_time
timezone_cz_time

@mairin
Copy link
Contributor

mairin commented Jan 16, 2024

Ah while we're here, sorry, one quick tweak if not a pain - a little more padding between the date dropdowns and the "/" characters inbetween the widgets... maybe just 6 px?

Like this? I tried it with two different locales (en_US and cs_CZ): timezone_us_time timezone_cz_time

@poncovka this looks fantastic - how do you feel about it?

The spoke doesn't look good without the timezone map, so do a little redesign.
See the discussion at rhinstaller#5355.
@poncovka
Copy link
Contributor Author

@poncovka this looks fantastic - how do you feel about it?

I like is as well. Ok. Let's do that :-)

@poncovka
Copy link
Contributor Author

/build-image --boot.iso

Copy link

Images built based on commit d401391:

  • boot.iso: success

Download the images from the bottom of the job status page.

Copy link
Contributor

@rvykydal rvykydal 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 to me.

@jkonecny12
Copy link
Member

/kickstart-test --testtype smoke

@Conan-Kudo
Copy link
Contributor

I mourn the loss in fanciness, but it looks good otherwise.

@poncovka
Copy link
Contributor Author

/kickstart-test -t time

@poncovka poncovka merged commit 1433e88 into rhinstaller:master Jan 22, 2024
19 checks passed
@ni-balexand
Copy link

I mourn the loss in fanciness, but it looks good otherwise.

Indeed. I've enjoyed the ease-of-use behind just pointing and clicking. Scrolling through a long list of cities is annoying. The bigger problem I think, though, is that users who are not familiar with city-based time zones (i.e. don't know what city they should be associated with) might have a hard time finding the right one (e.g. a user in Austin needs to know to select Chicago).

This is not an improvement for the user, it's taking a step backwards. Why was this removed?

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.

7 participants