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.

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

ISSUE-198 - Reorganization of the plot of land interface, in particular for waste garbage cans #207

Merged
merged 15 commits into from
Jan 23, 2025

Conversation

LPoin
Copy link
Contributor

@LPoin LPoin commented Dec 31, 2024

Multiple changes to the parcel search interface:

  • the ‘+’ button is now placed after the tabs used to search for plots, like a web browser.
  • tab deletion is now done by a cross on each tab. The dustbin has been removed.
  • the ‘search’ and ‘delete’ buttons have been grouped together in the plot search block to avoid confusion with the plot selection block below.

These changes have been reflected in the search for owners and co-owners to maintain the overall consistency of cadastrapp.

Tested in integration.
Mapstore 2023.02
Node 16.20.2

@landryb
Copy link
Member

landryb commented Jan 2, 2025

deployed locally, renders like this:

image

strangely i cant select plots anymore in the search results. i can doubleclick on the plot line and it opens the info form, i can tick the 'select all' box, but the box on the plot line is inactive. thus i cant select the plot line to test the trash...

creating a new empty selection tab 2, coming back to the selection 1 tab and clicking on the cross to close selection tab 2 destroys the wrong selection tab (ie tab1, the one where i had a plot from a map selection)

Peek 02-01-2025 10-08

@LPoin
Copy link
Contributor Author

LPoin commented Jan 2, 2025

Both issues have been fixed.

@landryb
Copy link
Member

landryb commented Jan 3, 2025

It's better, but there's still an UX annoyance: if you close a tab which isnt displayed/selected, after confirming, you get a blank area, and you need to reselect the previously selected tab, while it shouldn't stop showing the currently selected tab..

Peek 03-01-2025 11-18

@pierrejego
Copy link
Member

It's better, but there's still an UX annoyance: if you close a tab which isnt displayed/selected, after confirming, you get a blank area, and you need to reselect the previously selected tab, while it shouldn't stop showing the currently selected tab..

Peek 03-01-2025 11-18 Peek 03-01-2025 11-18

This is a regression, it was working on previous version. This point has to be corrected before merging

confirmContent={<Message msgId={'cadastrapp.search.confirmDeleteTab'}/>}
onClick={(e) => {
e.stopPropagation();
onTabDelete(!isDrop ? index : index + (MAX_TABS - 1));
Copy link

@Gaetanbrl Gaetanbrl Jan 8, 2025

Choose a reason for hiding this comment

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

You can also (other solution) just pass the delete func as property to reduce function complexity:

<DeletePlot delete={() => onTabDelete(index + (MAX_TABS - 1))}/>

Function :

function DeletePlot ({
    delete = () => {},
}) {    
    return (
        <OverlayTrigger placement="bottom" overlay={<Tooltip><Message msgId={'cadastrapp.search.deleteTab'}/></Tooltip>}>
            <ConfirmButton
                href="javascript:void(0)"
                confirmContent={<Message msgId={'cadastrapp.search.confirmDeleteTab'}/>}
                onClick={(e) => {
                    e.stopPropagation();
                    delete();
                    }}>
                <Glyphicon glyph="remove" />
            </ConfirmButton>
        </OverlayTrigger>
    )
}

Copy link

@Gaetanbrl Gaetanbrl left a comment

Choose a reason for hiding this comment

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

Minor not required change

@pierrejego pierrejego self-requested a review January 17, 2025 11:22
@pierrejego
Copy link
Member

Dans les tests effectués sur mapstore 2023-02-xx ( chrome et firefox ) , pour l'onglet "Compte propriétaire" les boutons "effacer" et "rechecher" sont cachés et donc pas accessible.

Sur les onglets "Nom d'usage ou de naissance" et Par lot les boutons rechercher ne fonctionne pas, refonctionne quand on recharge la page

@pierrejego pierrejego added this to the v2.2.2 milestone Jan 17, 2025
@pierrejego
Copy link
Member

Correction ok pour le bouton rechercher et l'adapation à la hauteur pour la recherche,

Par contre ça serait bien si le tableau de résultat s'adaptait aussi à la taille restante, pour ne mettre une scroll bar dans le tableau que si vraiment nécessaire et maximiser l'affichage des données avec l'espace disponible

image

const tabElement = document.querySelector(".plots-selection .tab-content");
parent.children.length > 2 ? tabElement.style.marginBottom = "50px" : tabElement.style.marginBottom = "";

const remainingHeight = parent.children.length > 2 ? Math.max(data.length * 35 + h3Height + ulSize + 30, 200) : Math.max(parentHeight - 220, 200); // 200px min
Copy link
Member

@pierrejego pierrejego Jan 22, 2025

Choose a reason for hiding this comment

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

See if 35 can be changed by a dynamic line height
Add comments to explain 30 and 200 px values ( 200 replace old 230 existing fixed values ) 30 margin to see object number

@pierrejego pierrejego merged commit 8ac331b into georchestra:master Jan 23, 2025
@pierrejego
Copy link
Member

Ok Tested on Mapstore 2024-02.X and 2023-02-X

@landryb
Copy link
Member

landryb commented Jan 23, 2025

the .cadastrapp plotPanel class with the 10px padding added to the .geOrchestra .nav > li > a class is too much waste of space...

image

for consistency, it should align with the height/width of those tabs i think..

image

the fact that there's no scrollbar in the results table anymore is interesting,but it just shifts the scrollbar to the extension pane which feels weird too.. i suppose UX-wise, it should trigger some kind of pagination ?

@pierrejego
Copy link
Member

We will have a look at the tab height

For the scrollbar, there are lots of differents case in the panel. We did not wanted to have two scrollbar when search panel is too long like this one.

This point need to be study, to find the best UI. For me an other issue need to be done

image

@landryb
Copy link
Member

landryb commented Jan 23, 2025

For the scrollbar, there are lots of differents case in the panel. We did not wanted to have two scrollbar when search panel is too long like this one.

right, hadn't thought about that case.. i guess.. it makes sense then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants