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.

Sign up for GitHub

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

Jump to bottom

Nmc/style sheet changes #5776

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Nmc/style sheet changes #5776

wants to merge 2 commits into from
+39 −34

Conversation

TSI-parulsharma
Copy link

@TSI-parulsharma TSI-parulsharma commented Jun 7, 2023 •
edited
Loading

Uploading Stylesheet_Change.PNG…
Did changes related to style sheet.
Added global style sheet file style.qss
Styling toolbar and progress bar using style.qss

camilasan
camilasan requested changes Jun 11, 2023
View reviewed changes
resources.qrc Outdated Show resolved Hide resolved
src/gui/settingsdialog.cpp Outdated Show resolved Hide resolved
src/gui/folderwizard.cpp Outdated Show resolved Hide resolved
src/gui/application.cpp Outdated Show resolved Hide resolved
src/gui/wizard/owncloudwizard.cpp Outdated Show resolved Hide resolved
theme/Style/style.qss Outdated Show resolved Hide resolved
theme/Style/style.qss Outdated Show resolved Hide resolved
src/gui/application.h Outdated Show resolved Hide resolved
src/gui/wizard/owncloudwizard.cpp Outdated Show resolved Hide resolved
src/gui/wizard/owncloudwizard.cpp Outdated Show resolved Hide resolved
@camilasan camilasan enabled auto-merge June 29, 2023 15:16
mgallien
mgallien requested changes Jun 29, 2023
View reviewed changes
Copy link
Collaborator

@mgallien mgallien left a comment

Choose a reason for hiding this comment

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

see my inline comments

src/gui/application.cpp Outdated Show resolved Hide resolved
src/gui/wizard/owncloudwizard.cpp Show resolved Hide resolved
src/gui/wizard/owncloudwizard.cpp Show resolved Hide resolved
src/gui/wizard/owncloudwizard.cpp Show resolved Hide resolved
src/gui/folderwizard.cpp Outdated Show resolved Hide resolved
auto-merge was automatically disabled June 30, 2023 06:43

Head branch was pushed to by a user without write access

@TSI-parulsharma TSI-parulsharma force-pushed the nmc/StyleSheet_Changes branch 3 times, most recently from c2355c9 to 841fe9b Compare July 5, 2023 09:34
@mgallien
Copy link
Collaborator

mgallien commented Jul 7, 2023

@TSI-parulsharma sorry for the delay, this slipped out of my mind
we have a DCO check that is failing
see https://github.com/nextcloud/desktop/pull/5776/checks?check_run_id=14788692861
I can help if needed

@TSI-parulsharma
Remove stylesheet from cpp files and add objectname
cd966ad 
- Separated the set style sheet changes
- Removed progress bar stylesheet from accountsettings.cpp

Signed-off-by: Camila <[email protected]>
@camilasan camilasan enabled auto-merge July 24, 2023 13:31
@codecov
Copy link

codecov bot commented Jul 24, 2023

Codecov Report

Merging #5776 (cd966ad) into master (a727971) will decrease coverage by 0.14%.
The diff coverage is n/a.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #5776      +/-   ##
==========================================
- Coverage   60.08%   59.95%   -0.14%     
==========================================
  Files         145      145              
  Lines       18762    18762              
==========================================
- Hits        11274    11249      -25     
- Misses       7488     7513      +25

see 4 files with indirect coverage changes

@nextcloud-desktop-bot
Copy link

AppImage file: nextcloud-PR-5776-cd966ad04f85cf08df09cb713ee4bf583e1653a2-x86_64.AppImage

To test this change/fix you can simply download above AppImage file and test it.

Please make sure to quit your existing Nextcloud app and backup your data.

@tsdicloud
Copy link

Does anybody understand the purpose of this PR?
I think it missed the actual topic of more flexible theming...

Any idea how to proceed?

@camilasan
Copy link
Member

I think it missed the actual topic of more flexible theming...

how do you mean?

@tsdicloud
Copy link

I don't understand why individual changes from MagentaCLOUD should be upstreamed.
The stylesheet mechanism is maybe nice (although I am not completely convinced anymore as it is only a complicated version of and added QStyle class...

MagentaCLOUD should not upstream things that may need to change on demand, as it would then mean to always ask for an upstream change. Let's discuss this F2F as planned.

@memurats memurats deleted the nmc/StyleSheet_Changes branch September 30, 2024 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mgallien mgallien mgallien approved these changes

@camilasan camilasan Awaiting requested review from camilasan

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@TSI-parulsharma @mgallien @nextcloud-desktop-bot @tsdicloud @camilasan