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

Observable Preferences T (NameDisplayPreferences, MainTablePreferences, ColumnPreferences) #9535

Merged
merged 9 commits into from
Jan 10, 2023

Conversation

calixtus
Copy link
Member

@calixtus calixtus commented Jan 5, 2023

Follow up to #9493

  • 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 developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
  • 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.

@calixtus calixtus added type: code-quality Issues related to code or architecture decisions preferences status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers labels Jan 5, 2023
@JabRef JabRef deleted a comment from Siedlerchr Jan 8, 2023
@Siedlerchr
Copy link
Member

Seems to work fine.

@koppor
Copy link
Member

koppor commented Jan 9, 2023

It seems, this has to do with column ordering in the main table and in the global search. For the main table, it works great. For the search, column re-ordering leads to a hidden column:

  1. Reorder Title / Author (so that Title is last)
    image
  2. Restart
  3. Start Global Search. See "Title" being missing
    image

@koppor
Copy link
Member

koppor commented Jan 9, 2023

When starting the main branch, the column "Author/Title" is completely lost:

image

I think, we should nevertheless move forward with this and add fixes after this is merged?

@calixtus
Copy link
Member Author

calixtus commented Jan 9, 2023

Is the maintable fine? Since this pr is reducing code duplication with the search dialog and the main table, this is probably a bug in the search dialog and hopefully not caused by my changes 😅 . So I agree, a follow up issue (maybe "good first") with bugs in the global search dialog should be filed and adressed

@Siedlerchr
Copy link
Member

As far as I remember the columns in the search are just based on the main table but are stored separate. So they should be independent

@calixtus
Copy link
Member Author

So as both reviewers approved or voted to merge, im merging.

@calixtus calixtus merged commit 4161d64 into main Jan 10, 2023
@calixtus calixtus deleted the prefs_t branch January 10, 2023 21:05
Siedlerchr added a commit that referenced this pull request Jan 16, 2023
…ialog

* upstream/main:
  Bump xmlunit-core from 2.9.0 to 2.9.1
  Bump mockito-core from 4.11.0 to 5.0.0
  Bump xmlunit-matchers from 2.9.0 to 2.9.1
  Bump junit-platform-launcher from 1.9.1 to 1.9.2
  Squashed 'buildres/csl/csl-styles/' changes from 43566f2..50eea55b2c
  New translations JabRef_en.properties (Portuguese, Brazilian) (#9559)
  Changed the color of light-color-text and highlighted text in … (#9546)
  New translations JabRef_en.properties (Portuguese, Brazilian) (#9557)
  chore: improve debug output in powershell starter script (#9550)
  Show development information (#9555)
  Observable Preferences T (NameDisplayPreferences, MainTablePreferences, ColumnPreferences) (#9535)
@calixtus calixtus mentioned this pull request Feb 14, 2023
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
preferences status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers type: code-quality Issues related to code or architecture decisions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants