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

Gallery code refactor: replace the word 'modal' #3221

Closed
misaugstad opened this issue Apr 27, 2023 · 2 comments · Fixed by #3673
Closed

Gallery code refactor: replace the word 'modal' #3221

misaugstad opened this issue Apr 27, 2023 · 2 comments · Fixed by #3673

Comments

@misaugstad
Copy link
Member

Brief description of problem/feature

When Gallery was written, the expanded view that you get from clicking on a card was referred to as a "modal", but a modal is supposed to be something that prevents you from interacting with the other content on the page. This is clearly not the case here!

I'd like to rename Modal.js, and generally replace that word in the Gallery code. The best thing I've come up in the interim is "Expanded View", and that's how I've been referring to it in Github and in the code for a few months. How do we feel about changing Modal.js to ExpandedView.js or just Expanded.js? Other ideas for how you would refer to the more detailed view of the label when clicking on the cards?

@jonfroehlich
Copy link
Member

jonfroehlich commented Apr 27, 2023 via email

@misaugstad
Copy link
Member Author

@LeeJoh22 here's your next task! One thing to keep in mind is our recommended line length limit of 120 characters, since "ExpandedView" is longer than "Modal"! Make sure to start by playing with Gallery for awhile, clicking on every button so that you know what they all do and can ensure that everything is still working as intended after the refactor. Though you don't need to use the "City" dropdown; it will send you from your local dev env to a test server for the city you click on! Just be careful of that.

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

Successfully merging a pull request may close this issue.

3 participants