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.

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

Move PropertyPage, ConceptPage #2785

Merged
merged 1 commit into from Oct 22, 2017

Conversation

Projects
None yet
3 participants
@mwjames
Copy link
Contributor

mwjames commented Oct 22, 2017

This PR is made in reference to: #2784

This PR addresses or contains:

  • Just in case some activities in relation to #2784 are going to happen, I dusted off an old PR that moves the property and concept page to its own SMW\Page NS
  • It contains some minor refactoring and injects some of the required services using a factory
  • Adds some simple unit tests to assert the general functionality

This PR includes:

  • Tests (unit/integration)
  • CI build passed

@mwjames mwjames added this to the SMW 3.0.0 milestone Oct 22, 2017

@mwjames

This comment has been minimized.

Copy link
Contributor

mwjames commented Oct 22, 2017

@kghbln @s7eph4n

I added a minor design change which can be switched to but by default it still uses the old smw-table smw-property-page-results legacy (legacy class). Removing legacy switches to the new design and since I'm not sure it makes much difference or whether the new alignment is an improvement or not it is hidden from the "normal" user. Feel free to suggest a better design.

Legacy

image

New

image

@s7eph4n

This comment has been minimized.

Copy link
Contributor

s7eph4n commented Oct 22, 2017

It makes it better aligned with the rest of the page, so I think it is a good move. 👍

N.B. 1: At some point we could think about a general rework of the styling, e.g. aligning Pages with improper assignments and Pages using..., better use of "screen real estate", etc.
(A general SMW style overhaul might be a candidate for one of the Outreach Programs.)

N.B. 2: MW supports Less since 1.22. Would it make sense to switch?

@mwjames

This comment has been minimized.

Copy link
Contributor

mwjames commented Oct 22, 2017

N.B. 1: At some point we could think about a general rework of the styling, e.g. aligning Pages with improper

#2786

N.B. 2: MW supports Less since 1.22. Would it make sense to switch?

#2787

@mwjames mwjames merged commit 716e88f into master Oct 22, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@mwjames mwjames deleted the property-page branch Oct 22, 2017

mwjames added a commit that referenced this pull request Oct 22, 2017

@kghbln

This comment has been minimized.

Copy link
Member

kghbln commented Oct 22, 2017

Note that due to 6792db0 the new styling is default which is indeed the better approach. PR now documented here and there.

@mwjames mwjames referenced this pull request Dec 10, 2017

Merged

Add value filter to property page, refs 2785 #2878

2 of 2 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment