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.

Sign up for GitHub

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

Jump to bottom

[GEOS-10556] Cleanup of CSS, HTML DOM structure en rewrite of some CSS - New PR #6474

Merged
merged 7 commits into from Jan 16, 2023
Merged

[GEOS-10556] Cleanup of CSS, HTML DOM structure en rewrite of some CSS - New PR #6474

merged 7 commits into from Jan 16, 2023

Conversation

MichelGabriel
Copy link
Contributor

GEOS-10556

Replacing PR #5984

This is a new PR with the same code changes (cherry picked) as the PR it is replacing. The new PR has been made because the old one couldn't be rebased properly.

The old PR contains all the comments that were made.

Link to proposal: https://github.com/geoserver/geoserver/wiki/GSIP-213

Checklist

For core and extension modules:

  • New unit tests have been added covering the changes.
  • Documentation has been updated (if change is visible to end users).
  • The REST API docs have been updated (when changing configuration objects or the REST controllers).
  • There is an issue in the GeoServer Jira (except for changes that do not affect administrators or end users in any way).
  • Commit message(s) must be in the form [GEOS-XYZWV] Title of the Jira ticket.
  • Bug fixes and small new features are presented as a single commit.
  • Each commit has a single objective (if there are multiple commits, each has a separate JIRA ticket describing its goal).

MichelGabriel and others added 5 commits January 3, 2023 10:15
@MichelGabriel
[GEOS-10556] Cleanup of CSS, HTML DOM structure en rewrite of some CSS
47cf035 
Link to proposal: https://github.com/geoserver/geoserver/wiki/GSIP-213

First changes (not ready yet):
- remove `UL/LI` that were used for layouting
- add `bootstrap-utilities` for positioning
- use `bootstrap` classes for positioning
- add `bootstrap` custom variables
- remove 50% of hardcoded styling
- overall cleanup of unused (IE6/IE7) styles
- rewrite some 'strange' CSS styling
- make label for checkboxes clickable
- put checkbox + label on 1 line
- cleaned fieldset/legend structure
@MichelGabriel
[GEOS-10556] Cleanup of CSS, HTML DOM structure en rewrite of some CSS
e86afb8 
Link to proposal: https://github.com/geoserver/geoserver/wiki/GSIP-213

Second batch of changes:
- remove `UL/LI` that were used for layouting
- remove hardcoded styling in core
- cleaned fieldset/legend structure
- removed unused HTML code

Fixes:
- improved alignment of `i18n` (geoserver#5984 (review))
- hide toolbar for openlayers (geoserver#5984 (comment))
- align CRS inpu (geoserver#5984 (review))
- improved positioning of `clear` link (geoserver#5984 (comment))
- increase padding of table cells
- move help link to the right of a `<fieldset>` `<legend>` (geoserver#5984 (comment))

Todo:
- write guidelines
- check `display:none;`
- check `extension`
- check community modules
@MichelGabriel
[GEOS-10556] Cleanup of CSS, HTML DOM structure en rewrite of some CSS
34cce85 
Link to proposal: https://github.com/geoserver/geoserver/wiki/GSIP-213

Third batch of changes:
- hardcoded styles from all extensions

Todo:
- write guidelines
@MichelGabriel
[GEOS-10556] Cleanup of CSS, HTML DOM structure en rewrite of some CSS
10c3b71 
Link to proposal: https://github.com/geoserver/geoserver/wiki/GSIP-213

Documentation with guidelines for layout, sizing and spacing
@MichelGabriel @jodygarnett
[GEOS-10556] Cleanup of CSS, HTML DOM structure en rewrite of some CSS
9017b9f 
Link to proposal: https://github.com/geoserver/geoserver/wiki/GSIP-213

- Fix a typo in a variable name
- Update tabel (dutch) to table (English)
- Update doc/en/developer/source/programming-guide/wicket-pages/index.rst

Co-authored-by: Jody Garnett <jody.garnett@gmail.com>
@aaime
Copy link
Member

aaime commented Jan 3, 2023

Had a quick go over the top level pages (and layers and some stores). Most look good, found the following issues:

  • The WMS page does not load
org.apache.wicket.WicketRuntimeException: The component(s) below failed to render. Possible reasons could be that:
	1) you have added a component in code but forgot to reference it in the markup (thus the component will never be rendered),
	2) if your components were added in a parent container then make sure the markup for the child container includes them in <wicket:extend>.

1. [CheckBox [Component id = autoEscapeTemplateValues, page = org.geoserver.wms.web.WMSAdminPage, path = form:autoEscapeTemplateValues, type = org.apache.wicket.markup.html.form.CheckBox, isVisible = true, isVersioned = false]]

	at org.apache.wicket.Page.checkRendering(Page.java:666)
	at org.apache.wicket.Page.onAfterRender(Page.java:821)
	at org.apache.wicket.markup.html.WebPage.onAfterRender(WebPage.java:224)
	at org.apache.wicket.Component.afterRender(Component.java:919)
	at org.apache.wicket.Component.render(Component.java:2343)
	at org.apache.wicket.Page.renderPage(Page.java:1018)
	at org.apache.wicket.request.handler.render.WebPageRenderer.renderPage(WebPageRenderer.java:124)
	at org.apache.wicket.request.handler.render.WebPageRenderer.respond(WebPageRenderer.java:236)
  • The WFS page does not load:
Tag  '<wicket:extend>' (line 3, column 3) has a mismatched close tag at  '</li>' (line 67, column 3)
 MarkupStream: [markup = file:/home/aaime/devel/git-gs/src/web/wfs/target/classes/org/geoserver/wfs/web/WFSAdminPage.html
<html xmlns:wicket="http://wicket.apache.org/">
<head></head><body>
  <wicket:extend>
    <fieldset>
  • The Filter chain editor page (for the "web" filter chain, but also for rest , gwc and default) does not load:
ag  '<fieldset class="border-0">' (line 29, column 19) has a mismatched close tag at  '</ul>' (line 33, column 5)
 MarkupStream: [markup = file:/home/aaime/devel/git-gs/src/web/security/core/target/classes/org/geoserver/security/web/auth/SecurityVariableFilterChainPage.html
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"
    "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml"
    xmlns:wicket="http://wicket.apache.org/">
  • In the style editor, when moving to full screen, and switching to a layer with many attributes, the pane does not have a scroll (but to be honest, I believe it was like that already, feel free to ignore):

image

@MichelGabriel
Copy link
Contributor Author

@aaime Thank you for the review, I'm going to fix the errors

@MichelGabriel
Copy link
Contributor Author

I pushed some changes/fixes for the bugs mentioned above by @aaime

@jodygarnett
Copy link
Member

jodygarnett commented Jan 4, 2023 •
edited

Thanks for restarting @MichelGabriel - now is a good time and get this merged to main.
Last time I went through all the screens; is that the plan again this time?

@MichelGabriel
Copy link
Contributor Author

@jodygarnett it should be the same as before, I didn't add new things

@aaime
Copy link
Member

aaime commented Jan 16, 2023

Did one more round, couldn't find issues. So merging, before it bit-rots again 🤣
Thanks @MichelGabriel !

@aaime aaime merged commit 81d42d2 into geoserver:main Jan 16, 2023
@jodygarnett
Copy link
Member

Thanks @MichelGabriel (and @aaime for review)

dieterstueken pushed a commit to dieterstueken/geoserver that referenced this pull request Jan 17, 2023
@MichelGabriel @jodygarnett @dieterstueken
[GEOS-10556] Cleanup of CSS, HTML DOM structure en rewrite of some CS…
00eb350 
…S - New PR (geoserver#6474)

* [GEOS-10556] Cleanup of CSS, HTML DOM structure en rewrite of some CSS

Link to proposal: https://github.com/geoserver/geoserver/wiki/GSIP-213

First changes (not ready yet):
- remove `UL/LI` that were used for layouting
- add `bootstrap-utilities` for positioning
- use `bootstrap` classes for positioning
- add `bootstrap` custom variables
- remove 50% of hardcoded styling
- overall cleanup of unused (IE6/IE7) styles
- rewrite some 'strange' CSS styling
- make label for checkboxes clickable
- put checkbox + label on 1 line
- cleaned fieldset/legend structure

* [GEOS-10556] Cleanup of CSS, HTML DOM structure en rewrite of some CSS

Link to proposal: https://github.com/geoserver/geoserver/wiki/GSIP-213

Second batch of changes:
- remove `UL/LI` that were used for layouting
- remove hardcoded styling in core
- cleaned fieldset/legend structure
- removed unused HTML code

Fixes:
- improved alignment of `i18n` (geoserver#5984 (review))
- hide toolbar for openlayers (geoserver#5984 (comment))
- align CRS inpu (geoserver#5984 (review))
- improved positioning of `clear` link (geoserver#5984 (comment))
- increase padding of table cells
- move help link to the right of a `<fieldset>` `<legend>` (geoserver#5984 (comment))

Todo:
- write guidelines
- check `display:none;`
- check `extension`
- check community modules

* [GEOS-10556] Cleanup of CSS, HTML DOM structure en rewrite of some CSS

Link to proposal: https://github.com/geoserver/geoserver/wiki/GSIP-213

Third batch of changes:
- hardcoded styles from all extensions

Todo:
- write guidelines

* [GEOS-10556] Cleanup of CSS, HTML DOM structure en rewrite of some CSS

Link to proposal: https://github.com/geoserver/geoserver/wiki/GSIP-213

Documentation with guidelines for layout, sizing and spacing

* [GEOS-10556] Cleanup of CSS, HTML DOM structure en rewrite of some CSS

Link to proposal: https://github.com/geoserver/geoserver/wiki/GSIP-213

- Fix a typo in a variable name
- Update tabel (dutch) to table (English)
- Update doc/en/developer/source/programming-guide/wicket-pages/index.rst

Co-authored-by: Jody Garnett <jody.garnett@gmail.com>

* [GEOS-10556] Cleanup of CSS, HTML DOM structure en rewrite of some CSS

Fix for 3 rebase bugs [geoserver#6474 (comment)]

* [GEOS-10556] Cleanup of CSS, HTML DOM structure en rewrite of some CSS

Add a scrollbar to the layer attributes tab when the style editor is in fullscreen mode

Co-authored-by: Jody Garnett <jody.garnett@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@MichelGabriel @aaime @jodygarnett