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

Fixed add group button changing color after adding 10 groups #8392

Merged
merged 6 commits into from
Jan 17, 2022

Conversation

dimitrisdimos00
Copy link
Contributor

Moreover changed the text color from white to background color. Changed the method setNewGroupButtonStyle of class GroupTreeView. Also changed the GroupTree.css file. Deleted the inactive pseudoclass as it will not be used.

fixes #8051

  • Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

In these pictures we see the new button not changing after having 10+ groups:

new group button 2
new group button

…er changed the text color from white to background color. Changed the method setNewGroupButtonStyle of class GroupTreeView.

fixes JabRef#8051
@mlep
Copy link
Contributor

mlep commented Jan 5, 2022

Based on your screenshots, issue #8051 does not seem solved by the current state of your PR: the Add group button looks different from the other buttons of the panels.
For example, here for the light theme:
ksnip_20220105-173036
The Add group button is quite different from the button Settings of the OpenOffice/LibreOffice panel and the button Search of the Web search panel.
Similar issue with the dark theme.

…utton in GroupTree.css. Deleted method setNewGroupButtonStyle brcause the button is unstylized.

fixes JabRef#8051
…utton in GroupTree.css. Deleted method setNewGroupButtonStyle as the button is unstylized.

fixes JabRef#8051
@dimitrisdimos00
Copy link
Contributor Author

I made some commits. Please check if the color of the button is better now. Thanks!

@mlep
Copy link
Contributor

mlep commented Jan 6, 2022

Thank you @dimitrisdimos00 for tackling this issue.
I do not have the tools at hand to build JabRef, so, I cannot check directly 😞 .
Maybe another developer can check on this, or maybe you can provide the screenshots, as you did before?
Cheers.

@dimitrisdimos00
Copy link
Contributor Author

Here:
Στιγμιότυπο οθόνης 2022-01-06 124222
Στιγμιότυπο οθόνης 2022-01-06 124355

@mlep
Copy link
Contributor

mlep commented Jan 6, 2022

@dimitrisdimos00 This looks great!
Thank you for making JabRef a better software!

@dimitrisdimos00
Copy link
Contributor Author

@mlep Your welcome! Glad to help!

@Siedlerchr Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Jan 6, 2022
@ThiloteE
Copy link
Member

ThiloteE commented Jan 7, 2022

In my opinion, in light mode, the darkish blue colour (or at least some kind of different colour) is needed as a contrast to web search, which is right underneath. This might not be a problem, if #8398 is implemented.

@calixtus
Copy link
Member

calixtus commented Jan 9, 2022

I don't really know how to proceed with this PR.
We had some discussion a while ago about this feature (#8051 (comment)) and decided to include it. Removing this feature should not happen without a proper discussion. Ill put this on for next dev call.
Maybe you could write down some pros and cons here.

We also probably could use something like our ADRs we have for decisions about code architecture but for our UI design decisions?

@Siedlerchr
Copy link
Member

I never really got the idea of those after 10 groups it's showing in a different color

@dimitrisdimos00
Copy link
Contributor Author

Well personally I find it kind of strange as a functionality. If we want the user to learn this button maybe we could do it through a tutorial rather than this.

@tobiasdiez
Copy link
Member

I remember I got the idea from feedly (In http://blog.feedly.com/wp-content/uploads/2017/10/discover.gif the "Add content" button has a green background until you added a few sources). Given that their interface is quite usable, I trusted that they have competent ui experts and did their research whether this feature is helpful or not (we sadly don't have the resources for a proper A/B testing etc). But I do agree that it's not very standard behavior to change the background color like this.

@dimitrisdimos00
Copy link
Contributor Author

I guess it's not such a bad feature. If we keep it though it would be better to change it to a different, more visible and more matching color than before...

@koppor
Copy link
Member

koppor commented Jan 17, 2022

DIN 66234, Teil 8: "Erwartungskonformität". - I would translate it to "expectation conformance" - An application should behave as expected by the user. The difficult thing is to guess the expectations. Since this is the only place in JabRef with that functionality - and that functionality does not seem to be common.

However, if this functionality is used consistently within JabRef, it is maybe cool. Maybe, this is also a call for an "basic" and "advanced" mode of JabRef. #futurework

Thus, we remove this functionality for now.

@koppor koppor merged commit a377f4c into JabRef:main Jan 17, 2022
@koppor koppor removed the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Jan 17, 2022
@dimitrisdimos00 dimitrisdimos00 deleted the fix-for-issue-8051 branch January 17, 2022 22:37
Siedlerchr added a commit that referenced this pull request Jan 21, 2022
* upstream/main: (50 commits)
  New Crowdin updates (#8451)
  Fix library tab exception when saving prefs (#8450)
  Rename Groups interface into Groups (#8449)
  New Crowdin updates (#8445)
  update snap url
  Update bug_report.yml for 5.5
  Show development information\n\n+semver: minor
  Release v5.5
  Update journal abbrev list
  New Crowdin updates (#8439)
  Refresh example styles
  Squashed 'buildres/csl/csl-styles/' changes from 5563ccc..0237ae7
  Fixed add group button changing color after adding 10 groups (#8392)
  Bump slf4j-api from 2.0.0-alpha5 to 2.0.0-alpha6 in /buildSrc (#8438)
  Bump libreoffice from 7.2.3 to 7.2.5 (#8436)
  Bump org.openjfx.javafxplugin from 0.0.10 to 0.0.11 (#8437)
  Fix file directory preferences not respected (#8429)
  Refresh example styles
  Squashed 'buildres/csl/csl-locales/' changes from c38205618f..4a551a87c3
  Refresh example styles
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UI: Dark theme: "Add group" button seems inactivated
7 participants