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

Adjust dialog window structure #2725

Merged
merged 5 commits into from
Oct 13, 2024
Merged

Adjust dialog window structure #2725

merged 5 commits into from
Oct 13, 2024

Conversation

jevenski
Copy link
Contributor

@jevenski jevenski commented Oct 10, 2024

This commit addresses styling issue of the header part of dialog windows. It also fixes the 4th bug in this latest feedback: #2721 (comment) . This issue happens because the background images are 600px width, but our new dialog windows can grow over that width, so the extra part becomes a desert. So I cropped off the additional part of those background images, kept only the icon part, and used CSS background color to implement the same effect. For the Oblivion theme, a CSS gradient background is used to mimic the original background.

  • Changes are done to the header section, separating the background and the window icon to different levels of HTML.
  • Make theme adaptions, with the aime of decreasing CSS selector specificity and reducing code duplication.

- Changes are done to the header section, separating the background
  and the window icon to different levels of HTML.
- Make theme adaptions, with the aime of decreasing CSS selector
  specificity and reducing code duplication.
@stickz
Copy link
Collaborator

stickz commented Oct 10, 2024

@jevenski You can submit this change to master. If @koblack can test and confirm a resolution, I will push a hotfix.

@jevenski jevenski changed the base branch from develop to master October 10, 2024 16:53
@koblack
Copy link
Contributor

koblack commented Oct 10, 2024

Yes, I confirm it is fixed

image

@jevenski
Copy link
Contributor Author

This branch was created based on the develop branch, and then rebased to the master branch using Github mobile app. Is this going to be a problem? Also, I saw the develop branch is 6 commits behind the master branch, do we leave it as is and merge all the hot fixes when doing a version upgrade?

@stickz
Copy link
Collaborator

stickz commented Oct 11, 2024

This branch was created based on the develop branch, and then rebased to the master branch using Github mobile app. Is this going to be a problem? Also, I saw the develop branch is 6 commits behind the master branch, do we leave it as is and merge all the hot fixes when doing a version upgrade?

Hotfixes go to master for v5. Version 5.1 has started on the develop branch. I will cherry pick to develop then rebase when ready to mark v5.1 as stable.

If there's anything like the delete key fix you want to backport to v4.3, we can do that as well. I'm open to offering long term support for v4.3, until v5.1 becomes polished. Major providers have started reviewing v5. We're going to get a lot more issue reports soon!

@jevenski
Copy link
Contributor Author

I'm keeping an eye on the issue reports these days, and going over some old issues to see if I can fix some of those. We might need to clean up the reports a bit before new issues arise. For example, this one (third-party plugin issue): #2631, and this one (already merged to master): #2679.

@stickz
Copy link
Collaborator

stickz commented Oct 11, 2024

I'm keeping an eye on the issue reports these days, and going over some old issues to see if I can fix some of those. We might need to clean up the reports a bit before new issues arise. For example, this one (third-party plugin issue): #2631, and this one (already merged to master): #2679.

Done, I closed off those 2 issue reports and one more.

- Fix tab content being unable to scroll to bottom on mobile.
- Fix progress text displaying on top on the status bar.
@jevenski
Copy link
Contributor Author

The 2nd and the 5th issue have been resolved with the latest two commits. It would be appreciated if @koblack can take the trouble to test them again. The S-Table is still not perfect yet, though, with some issues like body column width unaligned with the header. I'll try to polish that part in v5.1.

@koblack
Copy link
Contributor

koblack commented Oct 11, 2024

Thanks, I checked it and it's fine now

image

image

@jevenski
Copy link
Contributor Author

Hi @stickz, what do you think about this one? Anything else I should modify in order to get this PR merged?

@stickz
Copy link
Collaborator

stickz commented Oct 13, 2024

Hi @stickz, what do you think about this one? Anything else I should modify in order to get this PR merged?

I'll review it tomorrow. I had a family gathering to attend tonight.
I just need to make sure everything works properly, before I click the green merge button.

@jevenski
Copy link
Contributor Author

Enjoy the precious time with family!
I still have one question though. Once this commit becomes a hot fix to the master branch, can I rebase the develop branch immediately onto this commit, so that we can continue developing v5.1 based on this fix and thus avoid merge conflicts when push out v5.1?

@stickz
Copy link
Collaborator

stickz commented Oct 13, 2024

Merged, going to do a release control (beta) version of v5.1. We will confirm everything works properly there.

@stickz stickz merged commit 5c83eb0 into Novik:master Oct 13, 2024
@jevenski jevenski deleted the flex-dialog branch October 13, 2024 14:40
@stickz stickz mentioned this pull request Nov 13, 2024
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants