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

Added randomly generated marker placement #966

Merged
merged 11 commits into from
Aug 5, 2023
Merged

Conversation

elad-haviv
Copy link
Contributor

@elad-haviv elad-haviv commented Jul 29, 2023

Description

  • I needed an option to generate random markers of specific types for my map.
  • Added new functionality to the "Markers generation settings" window.
  • The feature adds another column to the marker list with a button that enables the user to place a new randomly generated marker of that type on the map.

image

Type of change

  • Bug fix
  • New feature
  • Refactoring / style
  • Documentation update / chore
  • Other (please describe)

Versioning

  • Version is updated
  • Changed files hash is updated

@netlify
Copy link

netlify bot commented Jul 29, 2023

Deploy Preview for afmg ready!

Name Link
🔨 Latest commit 3c8c7bd
🔍 Latest deploy log https://app.netlify.com/sites/afmg/deploys/64ce0dbdbd0ae200084d044f
😎 Deploy Preview https://deploy-preview-966--afmg.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@elad-haviv elad-haviv marked this pull request as ready for review July 29, 2023 17:24
@elad-haviv elad-haviv changed the title Added randomly generated marker placement in "" Added randomly generated marker placement Jul 29, 2023
@Azgaar Azgaar self-requested a review July 30, 2023 10:17
versioning.js Outdated Show resolved Hide resolved
@Azgaar
Copy link
Owner

Azgaar commented Jul 30, 2023

Hello. It looks good! Should not take a lot of time to do the corrections and then I will test it additionally and merge.

@Azgaar
Copy link
Owner

Azgaar commented Jul 30, 2023

By the way, I am not sure it should be there in Markets generation settings. It's not a setting and not related to that menu. There is already a functionality to add a marker on click. But not it just creates a marker of the latest type. Can this button be reused to open a menu to add a marker of a selected type? I would be more semantically correct

@elad-haviv
Copy link
Contributor Author

elad-haviv commented Aug 4, 2023

By the way, I am not sure it should be there in Markets generation settings. It's not a setting and not related to that menu. There is already a functionality to add a marker on click. But not it just creates a marker of the latest type. Can this button be reused to open a menu to add a marker of a selected type? I would be more semantically correct

I added a selection field in the Markers Overview window.
When the user clicks on the standard '+' button, it generates a random marker of the selected type.
The default is value to 'empty,' retaining the button's current functionality (adding a '?' marker).
What do you think?

@Azgaar
Copy link
Owner

Azgaar commented Aug 4, 2023

Yes, it should be fine in general. It looks a bit clumsy as there is no space for it. Also type selection toggles the add button state, it's fine, but weird if you select something again and button is getting unselected

@elad-haviv
Copy link
Contributor Author

Yes, it should be fine in general. It looks a bit clumsy as there is no space for it. Also type selection toggles the add button state, it's fine, but weird if you select something again and button is getting unselected

I fixed it so type selection activates the add button state instead of toggling it.
also changed the selection menu to a button so it's less clumsy, is that okay?

@Azgaar
Copy link
Owner

Azgaar commented Aug 5, 2023

Yes, it looks good

versioning.js Outdated Show resolved Hide resolved
@Azgaar Azgaar merged commit e542574 into Azgaar:master Aug 5, 2023
4 checks passed
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.

2 participants