theotherp / nzbhydra2

Usenet meta search
Other
1.27k stars 76 forks source link

Layout issues #94

Open nemchik opened 6 years ago

nemchik commented 6 years ago
theotherp commented 6 years ago

Yeah, sorry, you pushed that issue when I was already working on v2 but wasn't yet sure how much I would keep.

nemchik commented 6 years ago

Thanks for getting that in. Navigation works now, and the logo fits the page properly, which fixed some other issues, but there's something else going on that I don't think v1 did (I could be wrong). http://i.imgur.com/1lXG6Sr.jpg

I might be able to have a look at this sometime next week.

nemchik commented 6 years ago

P.s. the remaining issue only really affects the settings pages. Searches, stats, etc all seem fine after the fixes you put in last night.

theotherp commented 6 years ago

On the downside it displays vertical and horizontal scrollbars on my computer.

nemchik commented 6 years ago

Well drats. Like I said I can look at the HTML/CSS sometime next week.

theotherp commented 6 years ago

Thanks :-)

nemchik commented 6 years ago

Ok, so I've looked a little bit (just on the surface) and some weird things are going on. I want to say they are coming from bootstrap but I honestly don't know.

For starters, the login screen has a horizontal scroll bar even on my desktop (with a huge resolution) and I've found this is because of a -15px margin that I was able to locate in multiple files, but from what I can tell you're using less which i'm not familiar with so I don't know if I'd be making the change in the wrong place.

Then after logging in there's various alignment issues on mobile where things just don't line up. Here's one example: https://github.com/theotherp/nzbhydra2/blob/master/core/ui-src/html/states/config.html#L155-L167 If you change that block to look like this

        <div class="col-md-6 config-content">
            <label for="{{::id}}" class="col-md-7 control-label config-label">
                {{ to.label }} {{ to.required ? "*" : ""}}
            </label>
            <formly-transclude></formly-transclude>
            <div class="my-messages" ng-messages="fc.$error" ng-if="options.formControl.$touched || form.$submitted" ng-messages-multiple>
                <div class="some-message has-error control-label" ng-message="{{::name}}" ng-repeat="(name, message) in ::options.validation.messages">
                    {{ message(fc.$viewValue, fc.$modelValue, this)}}
                </div>
            </div>
        </div>

This (and getting rid of those -15px margins all over the place) would immensely fix the view on mobile and not affect desktop at all. I'll keep looking for more things, but I don't know how much help I might be with direct edits or a PR to your repo since I'm just not familiar with less and the way you have things organized (I'll try though).

Once those two things are fixed I'll point out more.

theotherp commented 6 years ago

Hey, any help is appreciated. You can just tell me what to change. Or do a PR for the change in the CSS and I transfer it to less.

Am 02.02.2018 21:33 schrieb "Eric Nemchik" notifications@github.com:

Ok, so I've looked a little bit (just on the surface) and some weird things are going on. I want to say they are coming from bootstrap but I honestly don't know.

For starters, the login screen has a horizontal scroll bar even on my desktop (with a huge resolution) and I've found this is because of a -15px margin that I was able to locate in multiple files, but from what I can tell you're using less which i'm not familiar with so I don't know if I'd be making the change in the wrong place.

Then after logging in there's various alignment issues on mobile where things just don't line up. Here's one example: https://github.com/theotherp/ nzbhydra2/blob/master/core/ui-src/html/states/config.html#L155-L167 If you change that block to look like this

    <div class="col-md-6 config-content">
        <label for="{{::id}}" class="col-md-7 control-label config-label">
            {{ to.label }} {{ to.required ? "*" : ""}}
        </label>
        <formly-transclude></formly-transclude>
        <div class="my-messages" ng-messages="fc.$error"

ng-if="options.formControl.$touched || form.$submitted" ng-messages-multiple> <div class="some-message has-error control-label" ng-message="{{::name}}" ng-repeat="(name, message) in ::options.validation.messages"> {{ message(fc.$viewValue, fc.$modelValue, this)}}

This (and getting rid of those -15px margins all over the place) would immensely fix the view on mobile and not affect desktop at all. I'll keep looking for more things, but I don't know how much help I might be with direct edits or a PR to your repo since I'm just not familiar with less and the way you have things organized (I'll try though).

Once those two things are fixed I'll point out more.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/theotherp/nzbhydra2/issues/94#issuecomment-362698989, or mute the thread https://github.com/notifications/unsubscribe-auth/ANtAeeKayFnQPscbALqEw9Gs4F7A7tX4ks5tQ3ESgaJpZM4RvGtl .

nemchik commented 6 years ago

https://i.imgur.com/bLNCqsC.png

nemchik commented 6 years ago

That shows what files have the -15px margin. I'm pretty sure they can all be removed or replaced with 0.

nemchik commented 6 years ago

Ok, I'm working on a PR, but I would really like to discuss what you have going on with CSS. I see you have normalize.css in a few places, and it's a significantly older than the current version. I also see you are combining CSS into alllibs.css and stripping out new lines in all your CSS. So... why? I think you could alleviate a number of headaches by separating out the CSS (and probably JS, I'm guessing they are being combined as well) and formatting them properly instead of stripping out newlines. There's numerous benefits to this as well that I'm willing to discuss, but I'd like to know the reasons for why you've done things this way so far? You might have some golden insight that I don't and at the end of the day it might be better this way.

edit: I see the use of normalize.css is coming from bootstrap 3.x. I don't think this is an issue, but I would still definitely like to know why you're stripping newlines and combining all libraries.

theotherp commented 6 years ago

I think unifying all CSS files in one is common practice. It reduces the amount of file transfers (not really an issue here, to be honest) and makes the handling easier.

I haven't really spent much thought on the matter, most tutorials and archetype generators do it that way and I don't see a reason to change it.

I use wiredep do gather all the needed scripts and CSS files, concat them and them uglify them to reduce the file size. I never look at the CSS files themselves. Everything in alllibs.css comes from the libraries, everything in the three files grey.css, dark.css and bright.css is compiled, concatted, uglified LESS.

More or less the same goes for JS.

None of the files in the static folder are meant to be human readable. I only actively work on the files in https://github.com/theotherp/nzbhydra2/tree/master/core/ui-src

nemchik commented 6 years ago

so which file is actually used for the login? I see both core\src\main\resources\templates\login.html and core\ui-src\html\states\login.html and I'm wondering which one would be the right one to edit?

I've done a little more digging and found the -15px margins are actually a part of bootstrap, and so taking them out might affect a lot more than I originally thought. This also lead me to looking into why bootstrap is applying those margins in the places I'm seeing them, and it turns out the first example I've found is the login page lacks a .container div.

I imagine there are a number of other places where I may be able to help with the implementation of bootstrap in order to get things looking/working better.

theotherp commented 6 years ago

Ignore whatever you see in the resources folder. Focus on the ui-src folder.

The downside is that you won't be able to see the effects of whatever you do. There might be a way to get that working but it would require some code changes and for you to get gulp running, without it will be really hard for you.

nemchik commented 6 years ago

I've contributed to other repos that used things like grunt https://github.com/berrberr/streamkeys there's one such example. They included the files needed in the repo so that others could run a few quick commands and be able to test and contribute.

Not sure if gulp works the same way, but I don't mind doing some upkeep if you can provide instructions.

Once I dig in I'll be able to help with a lot of things, from minor fixes to major overhauls if needed (and time allows). I can certainly resolve the problems that lead me to open this issue, and then just let me know if there's anything else I can help with.

theotherp commented 6 years ago

To compile the source files you just have to:

  1. Install nodejs: https://nodejs.org/en/
  2. Go into the main folder of your NZBHydra 2 installation (where readme.md is located) and create a folder static
  3. Go to the core folder in the NZBHydra 2 git clone folder
  4. Install yarn: npm install -g yarn
  5. Install all dev dependencies: yarn install --ignore-engines
  6. Install bower components: ./node_modules/.bin/bower install
  7. Run ./node_modules/.bin/gulp --static <full path to the folder you created in step 2>

The LESS/HTML/JS files in ui-src are watched and whenever you change any of them the files are built and written to the folder static. The first time this may take 20 seconds or so, afterwards it should be relatively fast. Changes to LESS files may take a couple of seconds though.

With v1.3.3 which I will release later Hydra will look in the static folder first when any UI resources are queried. That way you can just reload Hydra in your browser and see the effects of your changes.

Note: The next time you just have to execute step 7 in the core folder.

theotherp commented 6 years ago

Hm, sorry, 1.3.3 will have to wait, suddenly my layout is fucked up and I don't have any time left to debug it.

theotherp commented 6 years ago

If you want to try to get started follow the instructions here: https://github.com/theotherp/nzbhydra2/wiki/Development-of-frontend-resources

I'll release 1.3.3 tomorrow, until then you can experiment a bit and see if you get gulp running.

nemchik commented 6 years ago

FYI https://github.com/theotherp/nzbhydra2/commit/89009c9ac8e59616c650ccb4b777d0a47fa28d9b#r27452241

I'm not sure that the change I submitted caused whatever issue you were having, but when I tested (editing the code in Chrome's developer console) that specific change seemed fine.

nemchik commented 6 years ago

Ok, now that i'm playing with 1.3.3 that change I just mentioned messes the whole config page up and I have no idea what's going on that's causing it.

theotherp commented 6 years ago

Yeah, you're right, I remember I noticed that too but then forgot about it

nemchik commented 6 years ago

It only took me a few minutes to find that first fix, but I played with it this morning for a good while and couldn't find a fix. My hunch is there's class attributes being nested in the wrong order, but I haven't been able to narrow it down.