Feature multiple banners

pamtbaau commented 2 years ago

JS:

When UI is opened, a banner with number of changed items is shown.
On each Publish,
- Banners are cleared
- A banner with result of Publish is shown + banner with number of available changed items
Banner type (info or error) has been moved into BannerType enum. The following BannerTypes are available:
- BannerTypes.info : purple
- BannerType.error: red
- BannerType.notice: green

SCSS:

Added bottom-margin: 1px below each banner

Note:

It might be necessary to compile/minify typescript and scss again after merging, since minified files are often single line. This will cause complete file overrides when merging.
But I can do that for you in case you have no npm setup (yet).

hughbris commented 2 years ago

Going into the project build directory and running shell script containing : docker run -it --rm -e "TERM=xterm-256color" -v $(pwd):/usr/src/app -w /usr/src/app node:18-alpine3.14 npm "$@" has given me a safe build tool! Tick that one off. Credit to Aaron Saray's blog.

$ my-npm install
$ my-npm run build

pamtbaau commented 2 years ago

It seems I've forgotten to change the target branch for this PR. It should have been 'pamtbaau/pushy-admin-requests3'... I've just corrected the target branch and there are no merge conflicts expected anymore.

All other PRs do correctly want to merge into 'pamtbaau/pushy-admin-requests3'.

Also note that the last 3 PRs are not successions on top of each other, but separate changes based on branch 'pamtbaau/pushy-admin-requests3'. Which means that this PR does not include #12 and #13.

hughbris commented 2 years ago

I see I have forgotten to change the target branch for this PR. It should have been 'pamtbaau/pushy-admin-requests3'...

All other PRs do want to merge into 'pamtbaau/pushy-admin-requests3'.

I noticed and had a strategy for it, but because I knew but still forgot what you point out below, it doesn't matter that you've changed it back.

Also note that the last 3 PRs are not successions on top of each other, but separate changes based on branch 'pamtbaau/pushy-admin-requests3'. Which means that this PR does not include #12 and #13.

I did this all wrong, so I might have to manually close these PRs as Github has not picked up the other two PRs, which were mistakenly merged into develop.

You did this right. I am merging them into develop. I merged pamtbaau/pushy-admin-requests3 into develop a couple of days ago but forgot to push that for you.

So on both counts, the mistakes are mine.

I will push up all three merged into develop shortly and that will be a good base branch for features.

pamtbaau commented 2 years ago

I will push up all three merged into develop shortly and that will be a good base branch for features.

OK, I will branch from 'develop' from now on.

So on both counts, the mistakes are mine.

I don't think so... Your merge into develop is from March 18, early in the morning (UTC+2) and my PR's in the afternoon. The merge is not in my local repo, so I probably forgot to pull all changes in first. My bad. This collaboration certainly has a learning curve...

You "British" (descendants) are so overly polite... Please let me know once I become a Moloch instead of an asset. :-)

hughbris commented 2 years ago

I merged the "merged develop" into this PR just before. All three features seem to work, but the build fails with

> pushy@1.0.0 build
> npm-run-all --parallel css js

ERROR: "js" exited with 243.

So I'm not sure if that's easily fixed or you can diagnose that. If you can, please update this feature branch with code that transpiles and I will then merge that into origin/develop. Then we can begin working off that again!

pamtbaau commented 2 years ago

I'll have a look.

pamtbaau commented 2 years ago

Minified files are difficult to merge, because they are one-liners. The merge has left merge conflict lines in these files.

<<<<<<< ours
#admin-main .titlebar .button-bar ... etc
||||||| base
#admin-main .titlebar .button-bar ... etc.
=======
#admin-main .titlebar .button-bar ... etc.
>>>>>>> theirs

Removing files /css/*.min.*, solved the issue. Minified files will be rebuild correctly again.

Added new commit

hughbris commented 2 years ago

I messed up the automatic PR resolution, but this is merged in now after a1c6b8e4977a52775fbef9582f97e4842b8d7199, so closing this. Great work!

pamtbaau commented 2 years ago

I must admit I as wondering about the strategy behind the merges... :-)

Commit messages like "Merge branch 'develop' into feature-*", doesn't seem logic to me...

Anyway, it seems branch 'develop' contains all the PRs (except PR #16 ) and I use 'develop' as parent for my feature branches.

Are you perhaps merging branches in your local repo instead of on Github?

hughbris commented 2 years ago

Those are Git generated messages. I can edit them of course but I thought leaving them alone might increase the chances of a successful PR merge being recognised by Github. I was merging develop into the feature after testing your branch to make sure it integrated properly because some of those were not based on develop or at least the current develop. I think so. It made sense at the time.

What you are doing now is the best approach.

hughbris / grav-plugin-pushy

Feature multiple banners #14