-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
fixed the disable and enable button position #10556
base: develop
Are you sure you want to change the base?
Conversation
@bhousel can you review my PR and if you seems it has some issue please tell me regarding this |
Using absolute positioning and a -532px margin is certainly one way to move the text, but it's not how I would do it. |
@bhousel ok i understand then please give me some time i can change my approach |
insert them into the document where you want them to be |
hey @bhousel i have changed the position of the footer without css but now it look like this but in this image map feature text and both button i.e(disable all and enable all) are not aligning in the same line. |
hey @bhousel what do you think which method i use as stated above ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @draunger, as Bryan mentioned, using a margin of -532px
isn't a particularly robust solution. It also has consqeuences for other components that use the same class name.
It would be more idiomatic to move div.feature-list-links
above ul.layer-feature-list
. You can find the relevant code here:
iD/modules/ui/sections/map_features.js
Lines 27 to 29 in ff8f044
var footer = containerEnter | |
.append('div') | |
.attr('class', 'feature-list-links section-footer'); |
@k-yle ok sending its commit using moving div.feature-list-links above ul.layer-feature-list |
Description
Approach
I have used position absolute property and margin property to set buttons.
Before the fix
After the fix
Attached issue
Closes #8878