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

XWIKI-19041: Replace the Livetable from DocumentsMacro with a Live Data macro #2775

Merged
merged 7 commits into from
Feb 2, 2024
Merged

XWIKI-19041: Replace the Livetable from DocumentsMacro with a Live Data macro #2775

merged 7 commits into from
Feb 2, 2024

Conversation

manuelleduc
Copy link
Contributor

@manuelleduc manuelleduc commented Jan 5, 2024 •
edited
Loading

Jira Url

https://jira.xwiki.org/browse/XWIKI-19041

Description of changes

  • Replace the use of the Livetable macro with a Live Data macro in XWiki.DocumentsMacro (the documents macro).
  • As a consequence, this is impacting all pages using the documents macro. This is expected to be transparent from an API pov
    • Main.SpaceIndex (linked from Main.Spaces)
    • the example in XWiki.DocumentsMacro
    • XWiki.Tableview for the page index
  • Most the changes are related to docker test page objects refactoring

Todo

Screenshots & Video

Space Index - Sandbox space

Before

image

After

image

Page Index

Before

image

After

image

Executed Tests

mvn clean install -Pquality,integration-tests,docker \
	-pl :xwiki-platform-index-ui,:xwiki-platform-index-test-docker,:xwiki-platform-index-test-tests,:xwiki-platform-index-test-pageobjects,:xwiki-platform-mail-test-docker,:xwiki-platform-mail-test-pageobjects,:xwiki-platform-livedata-test-pageobjects

@manuelleduc manuelleduc marked this pull request as ready for review January 17, 2024 09:23
@vmassol
Copy link
Member

vmassol commented Jan 17, 2024

To discuss:

  • About the description:
    • why in the LD and not in the page content
    • the font difference
    • the bad alignment
    • the integration with previous text in the page content
    • the desc to use for each LD
    • whether or not we want this always displayed
  • The vertical grey bars between the filters for the LD
  • Idea: Always have the Actions column as the right most column

Thanks!

@michitux
Copy link
Contributor

The vertical grey bars between the filters for the LD

They were introduced in #2475 and have been discussed extensively there, they are completely unrelated to the changes in this PR.

@manuelleduc
Copy link
Contributor Author

manuelleduc commented Jan 17, 2024 •
edited
Loading

Remove default description + allow the documents macro to define it own.
Change description style.

@vmassol
Copy link
Member

vmassol commented Jan 17, 2024

@michitux / @Sereza7 About discussing UI changes in PRs, this is not the best place to propose new UI changes. UI changes are complex and usually quite touchy. They need to be proposed on the forum. This morning, with Manuel, we've reviewed the before/after changes for the LD for this PR, and the vertical bars really look odd (and the last one even more so). Would be great if you could open a forum proposal about it (it seems urgent since the PR was pushed with that change already). Thanks

@manuelleduc
Copy link
Contributor Author

@vmassol following this morning call.

  • About the description:

    • why in the LD and not in the page content
    • the font difference
    • the bad alignment
    • the integration with previous text in the page content
    • the desc to use for each LD
    • whether or not we want this always displayed
  • Font diff fix on a separate commit (no special style anymore)
  • Bad alignment fixed
  • No generic description, a description parameter is introduced on the documents macro (directly forwarding the parameter to the eponymous parameter of the liveData macro)
    • no description for the page index
    • the text before the page index is now used as the livedata description
  • The vertical grey bars between the filters for the LD

Off-topic for this PR.

  • Idea: Always have the Actions column as the right most column

Done.

(screenshot to be updated in the following minutes)

surli
surli reviewed Jan 30, 2024
View reviewed changes
...ore/xwiki-platform-index/xwiki-platform-index-ui/src/main/resources/XWiki/DocumentsMacro.xml Outdated
properties="$stringtool.join($properties, ',')"
source="liveTable"
sourceParameters="$escapetool.url($sourceParameters)"
description="$!xcontext.macro.params.description"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the parameter is empty we don't get anything in the dom for the description right? i.e. it has 0 impact for backward compat' of the macro?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The old behavior was having a useless and generic "This table lists documents found on this wiki based on passed criteria. The columns can be sorted and some can be filtered." message in the dom (but hidden with a sr-only class) for all Livetable generated through the documents macro.
Now, a description is inserted only if passed explicitly to the description parameter of the documents macro.
This can be considered as a breaking change as the default message is not there anymore, but imo this is also an accessibility improvement as we remove some useless noise for users using screen readers.

@manuelleduc
XWIKI-19041: Replace the Livetable from DocumentsMacro with a Live Da…
fbbe90a 
…ta macro
@manuelleduc
XWIKI-19041: Replace the Livetable from DocumentsMacro with a Live Da…
2904e10 
…ta macro

Docker tests.
@manuelleduc
--wip-- [skip ci]
da1d6e0
@manuelleduc
--wip-- [skip ci]
c2daaae
@manuelleduc
XWIKI-19041: Replace the Livetable from DocumentsMacro with a Live Da…
f86896c 
…ta macro

- definition of a description parameter for the documents macro
- the generic description is not provided by default
  - no description provided for the page index
  - the text displayed before the space index table is now used as the description (also fix the alignemnt issue)
- likes column now before the actions (i.e., actions always last)
@manuelleduc
XWIKI-19041: Replace the Livetable from DocumentsMacro with a Live Da…
c821d6f 
…ta macro
@manuelleduc manuelleduc merged commit af6b5ab into xwiki:master Feb 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@surli surli surli left review comments

@michitux michitux michitux requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@manuelleduc @vmassol @michitux @surli