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

Correct some CSS issues remaining on UI #2586

Merged
merged 12 commits into from
May 19, 2019

Conversation

Nilvus
Copy link
Contributor

@Nilvus Nilvus commented May 18, 2019

A new set of CSS improvements. This PR should resolve some remaining issues :

I also reduce size on some dropdown button arrows, especially on filter collection (I also reduce height size on entry item near this dropdown) and display dropdown on filmic that was way too big.

@aurelienpierre : could you see if it's ok for you as it change some of your choices ?

Before merging that, please test as much as you can. I made lot of reviews tests and compare with actual themes what change and if this new css breaks other part. I don't find anything on lot of modules I test, even on map and print parts (I don't test capture part as I don't have a camera compatible).

As I change lot of things, I want to be sure that nothing is broken on the UI.

@TurboGit TurboGit self-requested a review May 18, 2019 17:23
@TurboGit TurboGit self-assigned this May 18, 2019
@TurboGit TurboGit added the bugfix pull request fixing a bug label May 18, 2019
@TurboGit TurboGit added this to the 2.8 milestone May 18, 2019
data/themes/darktable.css Outdated Show resolved Hide resolved
@edgardoh
Copy link
Contributor

@Nilvus , looks the same to me, something else than building it that I should do?

@Nilvus
Copy link
Contributor Author

Nilvus commented May 18, 2019

@Nilvus , looks the same to me, something else than building it that I should do?

That's not really looks the same to me, but that's not the same as right panel. I've made lot of tests without success. What I propose to you at first yesterday break other dialog box. But I've just had another idea that should much better. See new commit.

@Nilvus
Copy link
Contributor Author

Nilvus commented May 18, 2019

Sorry for two last commit. I forget something and didn't understand on resolving that I was just merging master css... I will stop for tonight.

@Nilvus
Copy link
Contributor Author

Nilvus commented May 19, 2019

Issue #2481 now corrected.

@Nilvus
Copy link
Contributor Author

Nilvus commented May 19, 2019

@edgardoh : could you test last updates of css. What do you think about these changes, especially prefs entries (and filter collection one) as it's related to your issue.

@TurboGit
Copy link
Member

@Nilvus : sounds like you are correct and I'm wrong:

https://www.w3.org/Style/Examples/007/units.en.html

pt should not be used for screen and px should be preferred.

@Nilvus
Copy link
Contributor Author

Nilvus commented May 19, 2019

By now, I hope this last commit will end this PR. I have just improve contrast with revert back grey_55 to default css color. That make a better contrast especially on filmstrip (difficult to make it better without breaking things).

I also clean grey css as it have redundant lines with darktable.css related theme. A comment needs to be more precise so I updated it.

@Nilvus
Copy link
Contributor Author

Nilvus commented May 19, 2019

@TurboGit : anyway I also found that 1pt=1.33px, so I can't change just the unit. For example, I have replaced font-size of blending tabs from 8pt to 8 px and see that line becoming too small. But changing it to 0.8em (or 10px or 11px, as 8pt = 10.64px) make it better.
By now, I let existing pt as it is (rendering si the most important part of that).

@Nilvus
Copy link
Contributor Author

Nilvus commented May 19, 2019

As I hope UI is better now, see screenshots of new main updates to help review :

Filter collection entry and arrow dropdown
filtercollection

Preferences dialog window
preferenceswindow

More condensed darkroom history
history

New live sample (main work by @TurboGit on another commit, just improve text lisibility)
livesamples

More reasonable size of display dropdown arrow on filmic
filmicarrow

@Nilvus Nilvus changed the title [WIP] Correct some CSS issues remaining on UI Correct some CSS issues remaining on UI May 19, 2019
@edgardoh
Copy link
Contributor

@Nilvus , much better. Still the controls (text entry, check box) could be smaller (in height), also the space between controls. But if can't be done I can live with it.

@Nilvus
Copy link
Contributor Author

Nilvus commented May 19, 2019

@Nilvus , much better. Still the controls (text entry, check box) could be smaller (in height), also the space between controls. But if can't be done I can live with it.

You will have to forgot having a smaller height as the window is a grid, I could only reduce height with margin and padding already to 0. And negative values are not recognized. So for that, it's not possible without a Gtk rework...

For space, it's possible by reducing margin from this part :

#preferences_notebook scrolledwindow label,
#preferences_notebook scrolledwindow entry,
#preferences_notebook scrolledwindow button
{
  padding: 0px;
  margin: 5px 0;
}

If I reduce from 5px to 3px, see new spacing (keep quite aerate would be better so I'm not on to reduce spacing more) :

Capture-20190519181316-800x830

@Nilvus Nilvus changed the title Correct some CSS issues remaining on UI [WIP] Correct some CSS issues remaining on UI May 19, 2019
@Nilvus
Copy link
Contributor Author

Nilvus commented May 19, 2019

Finally, prefs window is probably better with reducing space as last screen capture. So committed with a darktable-elegant code cleanup (many lines redundant from darktable default css and some lines forgotten and not needed, ie live-sample-data better with darktable default settings).

I also changed font-weight from 400 to normal to have the same setting (400=normal on css) on css.

@Nilvus Nilvus changed the title [WIP] Correct some CSS issues remaining on UI Correct some CSS issues remaining on UI May 19, 2019
Copy link
Member

@TurboGit TurboGit left a comment

Choose a reason for hiding this comment

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

Looks perfect to me! Thanks.

@TurboGit TurboGit merged commit c57a56c into darktable-org:master May 19, 2019
@Nilvus Nilvus deleted the css_tweaks branch May 19, 2019 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix pull request fixing a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants