Use bootstrap4 #234
Use bootstrap4 #234
Conversation
|
Can you do a minimal change please to upgrade to bs4? |
Codecov Report
@@ Coverage Diff @@
## master #234 +/- ##
==========================================
- Coverage 74.61% 70.86% -3.76%
==========================================
Files 42 43 +1
Lines 2226 2344 +118
==========================================
Hits 1661 1661
- Misses 565 683 +118
Continue to review full report at Codecov.
|
minimal means, doesn't care bad view by convert? e.g. looks like some css class (e.g. page-header) was removed from bootstrap4, so without implementing replacement of page-header, it becomes plain text. |
|
Well, I see some changes that do not look minimal, like: Size changes Colours are being added here and there, why? Some stuff looks like new features, like:
In a pull request that upgrades bootstrap to a new version, there should be only changes that make the UI work with bootstrap 4. Regression fixes related to that upgrade are also ok (so it does not look obviously bad / wrong if it looked ok with bs3). But other than that, there should be no other changes within this PR. Improvements on looks and usability are welcome in general, but should be separate. |
ah, it was why separated patches (can revert easily). but you wanted separated PR somewhat. ok, will separate. |
|
Added screen shot script to see UI changes. screenshots-before.tar.gz screenshots-before == bootstrap3 those should make easier to review. with result of screenshots, adjusted button wrap in display view, and tweaked QR view to be more bootstrap3 |
src/bepasty/tests/screenshots.py
Outdated
| raise ValueError("Can't login!!! Create a user 'foo' with the permissions" | ||
| "'read' and 'create' in your PERMISSIONS in the config") |
There was a problem hiding this comment.
not "user", it is a "secret".
So it becomes:
Can't login! Please edit your config, go to PERMISSIONS setting and add a new secret 'screenshots' with 'read' and 'create' permissions.
|
Looks good! One minor issue to fix and it's ready to merge! |
|
I just packaged and uploaded xstatic-bootstrap 4.5.3.1 to pypi (and also updated all other dependencies, see #235 ). Can you please test it and check again if bepasty looks ok? |
|
changed
|
|
tested with latest xstatic stuff. only noticed change looks like changed
previous
|
| raise ValueError("Can't login! Please edit your config, go to PERMISSIONS setting " | ||
| "and add a new secret 'foo' with all permissions.") |
There was a problem hiding this comment.
can you use a better secret than 'foo' here? maybe 'for-screenshots'?
There was a problem hiding this comment.
'foo' is default entry in config.py. and it is what test_website.py uses too.
changing from 'foo', just will make inconvenient to test.
There was a problem hiding this comment.
oh, ok. so keep it for now.
This tried to be similar with current look with bootstrap4.
To use this, for example
bepasty.conf should include this PERMISSIONS config.
PERMISSIONS = {
'foo': 'admin,list,create,read,delete',
}
run,
$ BEPASTY_CONFIG=$(pwd)/bepasty.conf bepasty-server
$ cd bepasty-server/src/bepasty/tests
$ pytest -s -vv screenshots.py
this makes "screenshots/" directory for screenshot images.
|
tested with latest xstatic stuff. only noticed change looks like changed
previous
|
|
OK, looks like this is finished! Will merge... |
This migrate bootstrap3 to bootstrap4. With this, IMO, becomes better views (but im not a web developer).
however, testing and checking UI is hard (no auto test, and people has own favorite), so this PR would be needed carefully review more or less.