Position the asterisk inside its parent, #908

fulv · 2019-06-05T18:54:25Z

instead of fixing it on the page.

mister-roboto · 2019-06-05T18:54:31Z

@fulv thanks for creating this Pull Request and help improve Plone!

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass.

Whenever you feel that the pull request is ready to be tested, either start all jenkins jobs pull requests by yourself, or simply add a comment in this pull request stating:

@jenkins-plone-org please run jobs

With this simple comment all the jobs will be started automatically.

Happy hacking!

davilima6 · 2019-06-05T19:12:43Z

Thanks, @fulv! I didn't test yet but it makes more sense. One thing, I think we need a new changelog entry instead of editing the existing one because that change was already released. You can replace that file edition by a new file 908.bugfix (you may use this PR's number instead of creating a new issue for it, which sounds worthy, as it's minor)

fulv · 2019-06-05T20:56:18Z

@davilima6 thanks, I've replaced the change to 895.bugfix with 908.bugfix.

davilima6 · 2019-06-05T21:11:57Z

I'd say it's ready for merge once it's green (you can use one of the squash options in Github UI).

Next, a corresponding PR would have to be made in plone.staticresources (we should automate that):

Clone buildout.coredev, edit its checkouts.cfg adding plone.staticresources and run bin/buildout -N
Fire up the instance (./bin/instance fg) and create a Plone site with site id = Plone, then shut it down.
Make sure 895-fix-alignment branch is checked out in src/mockup
Run ./bin/plone-compile-resources --bundle=plone
Change directory to src/plone.staticresources
Checkout a new branch (usually same name as the one from mockup)
Commit, push, open PR
Get PR number and add changelog file for towncrier, using commit message from previous step
Commit, push, trigger Jenkins Jobs and mark reviewers (probably same as mockup's PR)
(hopefully no need to cancel previous running CI job: if it didn't start yet it will be auto-cancelled)

... if you want I can do that :)

Should we add these steps to plone.staticresources documentation or somewhere more visible?

coveralls · 2019-06-05T21:24:38Z

Coverage decreased (-0.03%) to 57.527% when pulling deae9b2 on 895-fix-alignment into 4b696ba on master.

fulv · 2019-06-05T21:45:07Z

@davilima6 You've lost me at the first point. Which repo is checkouts.cfg in?

fulv · 2019-06-05T21:47:35Z

buildout.coredev, perhaps?

davilima6 · 2019-06-05T21:47:59Z

Yes, sorry about that. I edited above now.

fulv · 2019-06-05T22:26:57Z

./bin/plone-compile-resources --bundle=plone resulted in the following error:

Using site id: Plone
Target compile path: fetch from bundles
Traceback (most recent call last):
  File "/usr/local/plone-5.1/buildout.coredev/parts/instance/bin/interpreter", line 272, in <module>
    exec(_val)
  File "<string>", line 1, in <module>
  File "/usr/local/plone-5.1/buildout.coredev/src/plone.staticresources/src/plone/staticresources/_scripts/_generate_gruntfile.py", line 53, in <module>
    current = current[part]
  File "/usr/local/plone-5.1/buildout.coredev/eggs/Zope-4.0b10-py2.7.egg/OFS/ObjectManager.py", line 823, in __getitem__
    raise KeyError(key)
KeyError: 'Plone'
Traceback (most recent call last):
  File "bin/plone-compile-resources", line 291, in <module>
    sys.exit(plone.staticresources._scripts.compile_resources.main())
  File "/usr/local/plone-5.1/buildout.coredev/src/plone.staticresources/src/plone/staticresources/_scripts/compile_resources.py", line 148, in main
    base_path, args.instance, args.site_id, args.compile_dir
  File "/usr/local/plone-5.1/buildout.coredev/src/plone.staticresources/src/plone/staticresources/_scripts/compile_resources.py", line 57, in generate_gruntfile
    subprocess.check_call(cmd, env=os.environ)
  File "/usr/lib/python2.7/subprocess.py", line 541, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['/usr/local/plone-5.1/buildout.coredev/bin/instance', 'run', '/usr/local/plone-5.1/buildout.coredev/src/plone.staticresources/src/plone/staticresources/_scripts/_generate_gruntfile.py']' returned non-zero exit status 1

davilima6 · 2019-06-05T22:30:07Z

Another missing step: you have to fire up the instance, create a Plone site and stop the instance before running that command.

fulv · 2019-06-05T23:40:25Z

@davilima6 unrelated, but since I now know how to do this, I wanted to also fix plone/Products.CMFPlone#2490. The fix is another one-liner in the same .less file, but I can't figure out how to get my change to show up in my local Plone instance. I'm using the same buildout.coredev buildout as for this PR, so I have src/mockup with my branch, src/plone.staticresources, and I do ./bin/plone-compile-resources --bundle=plone, but when I launch the instance, and even recreate a new Plone site, my change is not reflected. What am I missing?

davilima6 · 2019-06-06T06:04:06Z

My bad. Structure Pattern is registered to plone-logged-in bundle, so command should be:

./bin/plone-compile-resources --bundle=plone-logged-in

To find that out you may:

Enable Development mode in http://localhost:8080/Plone/@@resourceregistry-controlpanel
Enable Debug CSS for plone-logged-in bundle and then Save button
Refresh http://localhost:8080/Plone/folder_contents

Or you can see the register in:

https://github.com/plone/plone.staticresources/blob/master/src/plone/staticresources/static/plone-logged-in.less

Instead of:

https://github.com/plone/plone.staticresources/blob/master/src/plone/staticresources/static/plone.less

Anonymous vs Logged-in Patterns

LESS Patterns

Based on the links above, considering LESS-only, the current list of anonymous patterns is:

mockup-patterns-autotoc
mockup-patterns-livesearch
mockup-patterns-markspeciallinks
mockup-patterns-modal
mockup-patterns-pickadate
mockup-patterns-select2

And authenticated ones are:

mockup-patterns-querystring
mockup-patterns-recurrence
mockup-patterns-relateditems
mockup-patterns-structure
mockup-patterns-tinymce
mockup-patterns-upload
plone-patterns-toolbar

JS Patterns

However for the final, complete Anonymous vs Logged-in Pattern List, it's best to look at the registered JS patterns:

For anonymous:

jquery
pat-registry
mockup-patterns-base
mockup-patterns-autotoc
mockup-patterns-contentloader
mockup-patterns-cookietrigger
mockup-patterns-formautofocus
mockup-patterns-formunloadalert
mockup-patterns-livesearch
mockup-patterns-markspeciallinks
mockup-patterns-modal
mockup-patterns-moment
mockup-patterns-pickadate
mockup-patterns-navigationmarker
mockup-patterns-preventdoublesubmit
mockup-patterns-select2
bootstrap-collapse
bootstrap-dropdown
bootstrap-tooltip

For logged-in:

mockup-patterns-inlinevalidation
mockup-patterns-querystring
mockup-patterns-recurrence
mockup-patterns-relateditems
mockup-patterns-structure
mockup-patterns-structureupdater
mockup-patterns-textareamimetypeselector
mockup-patterns-tinymce
plone-patterns-portletmanager
plone-patterns-toolbar

Maybe that's useful in docs too. I wish this info was more surfaced, for example, visible in Resource Registries UI.

fulv · 2019-06-06T20:49:55Z

@davilima6 great, thanks! At least now I have a little more confidence that my fix actually made it into both PRs.
What is the bus factor here? I.e., how many people other than you (and I guess now me) know how to build this stuff?
Of course it should be documented! I'm just not very familiar with the whole forest of trees here to offer any suggestions on where it should best be done. I suppose any documentation should be as close to the crime scene as possible, both to ensure it will be found by someone who doesn't know where to look, and also to make it more likely that it will be updated when something changes from underneath.

davilima6 · 2019-06-07T15:30:38Z

@fulv, I'll try to help remotely the Buschenschank Sprint to finish plone/plone.staticresources#13 but feel home to add some commits there too :)

davilima6 · 2019-06-07T15:36:41Z

@thet, @jensens, @ale-rt: It'd be nice to automate the updating of plone.staticresources on the creation of PRs in Mockup: #908 (comment) What would the best approach? A Jenkins jobs that opens PRs? A Makefile target (inside mockup?) that compiles the needed bundle(s) locally?

jensens · 2019-06-08T09:18:09Z

I cc also @gforcada as our CI expert.

+1 for all merge, documentation and some kind CI (semi) automation if this is possible without spending too much time.

gforcada · 2019-06-09T21:25:57Z

A script somewhere that those all the nuances is probably easier, specially permission wise and when to trigger it and what not.

Once that script has been tested a few times, it should be ready to get automated on CI IMHO.

fulv added 2 commits June 5, 2019 11:44

Position the asterisk inside its parent,

fa93717

instead of fixing it on the page.

Update 895.bugfix

139d043

fulv requested a review from ale-rt June 5, 2019 18:56

fulv added the 24 status: ready label Jun 5, 2019

fulv added 3 commits June 5, 2019 13:49

Update 895.bugfix

bb9ebb7

Update 895.bugfix

0b68121

Create 908.bugfix

deae9b2

fulv requested a review from davilima6 June 5, 2019 20:55

jensens merged commit 92c8795 into master Jun 10, 2019

jensens deleted the 895-fix-alignment branch June 10, 2019 08:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Position the asterisk inside its parent, #908

Position the asterisk inside its parent, #908

fulv commented Jun 5, 2019

mister-roboto commented Jun 5, 2019

davilima6 commented Jun 5, 2019

fulv commented Jun 5, 2019

davilima6 commented Jun 5, 2019 •

edited

coveralls commented Jun 5, 2019

fulv commented Jun 5, 2019

fulv commented Jun 5, 2019

davilima6 commented Jun 5, 2019

fulv commented Jun 5, 2019

davilima6 commented Jun 5, 2019

fulv commented Jun 5, 2019

davilima6 commented Jun 6, 2019 •

edited

fulv commented Jun 6, 2019

davilima6 commented Jun 7, 2019

davilima6 commented Jun 7, 2019

jensens commented Jun 8, 2019

gforcada commented Jun 9, 2019

Position the asterisk inside its parent, #908

Position the asterisk inside its parent, #908

Conversation

fulv commented Jun 5, 2019

mister-roboto commented Jun 5, 2019

davilima6 commented Jun 5, 2019

fulv commented Jun 5, 2019

davilima6 commented Jun 5, 2019 • edited

coveralls commented Jun 5, 2019

fulv commented Jun 5, 2019

fulv commented Jun 5, 2019

davilima6 commented Jun 5, 2019

fulv commented Jun 5, 2019

davilima6 commented Jun 5, 2019

fulv commented Jun 5, 2019

davilima6 commented Jun 6, 2019 • edited

Anonymous vs Logged-in Patterns

LESS Patterns

JS Patterns

fulv commented Jun 6, 2019

davilima6 commented Jun 7, 2019

davilima6 commented Jun 7, 2019

jensens commented Jun 8, 2019

gforcada commented Jun 9, 2019

davilima6 commented Jun 5, 2019 •

edited

davilima6 commented Jun 6, 2019 •

edited