BGDIINF_SB-2680 : remove ButtonWithIcon component#373
Conversation
385eb4b to
a8af77e
Compare
|
I haven't looked at he code yet, but just some comments while testing the site: Also previously the saving status had a reserved place in the drawing toolbox. Now the drawing toolbox becomes smaller or bigger as the saving status appears or disappears. |
a8af77e to
d5f0ea7
Compare
ltshb
left a comment
There was a problem hiding this comment.
On mobile the drawing close button is not well placed:
When clicking on it we can see that the cross is not centered on the button.
Personally I don't really like big buttons, I would limit their size based on their content:
Before the drawing tool selection button and delete last point where not so wide, I like it better, but this might be just a preference taste 😉
I haven't looked at the code yet.
d5f0ea7 to
1c0d5fb
Compare
|
I've added back the reserved space for the "Saving..." / "Map saved." message, it doesn't wobble anymore when showing it the first time this way. Regarding button size @ltshb I wanted to go back to something more standard, Bootstrap wise, and wanted to impose a no custom CSS rule. I also wanted to cover as many space as possible on mobile, as I found the previous button lackluster in space and difficult to touch. |
|
@pakb I see your point, all good for mobile. However now with this PR the buttons on Desktop are quite much bigger, for Desktop I would reduce the size of the drawing button (icon and text), this would also reduce the size of the menu and let more space for the map. With this PR the size of drawing button are slightly bigger on mobile, which is totally fine there, but are also bigger on desktop, and more there as on mobile. |
e9e8571 to
21e700f
Compare
|
There is still a mismatch between phone and desktop mode. This is probably due to using directly the bootstrap viewport size classes instead of "@include respond-above(phone)" or using isPhoneMode.
|
and replace all usage of this component with standard Bootstrap button. Also simplifying PopoverButton component, so that the specialized version used by feature editing is stored there, and the simpler version is use as plain HTML/JS by the time selector. Reworking the drawing toolbar so that it is more Bootrap based, and less custom CSS based. Also replacing many instances of custom CSS code in component, that could be tackled by Bootstrap, with Bootstrap classes.
Center the "X" close button in its space Only switch to phone layout on XS breakpoint (was switching on SM, and tablet layout was all messed up)
buttons were taking too much space with the display grid on desktop I tried to make them as squared as possible on desktop with some padding and min-width
for drawing toolbox, as it was doing some weird breakpoint interpretation when holding the phone on landscape mode, switching to the desktop layout of the toolbox without completely doing so. So I removed all the logic that was switching based on Bootstrap breakpoint and replaced it with v-if or v-class based approach based on the store values.
21e700f to
4b4464e
Compare
|
I've reverted back some layout choice from the drawing toolbox to using |
ltshb
left a comment
There was a problem hiding this comment.
Overall very good job 👍🏼
Just a minor issue with tippy.
Also not related to this PR but just an idea, the time button is quite big (was already before) it might be nicer to have a little bit smaller ?
src/modules/menu/components/activeLayers/MenuActiveLayersListItemTimeSelector.vue
Show resolved
Hide resolved
Adding small margin on the Y axis for the "Map saved..." text as it was a bit too close to buttons Removing unnecessary code and destroying tippy instance before unmount
and replace all usage of this component with standard Bootstrap button. Also simplifying PopoverButton component, so that the specialized version used by feature editing is stored there, and the simpler version uses plain HTML/JS for the time selector.
Reworking the drawing toolbar so that it is more Bootrap based, and less custom CSS based. Also replacing many instances of custom CSS code in component, that could be tackled by Bootstrap, with Bootstrap classes.
Test link