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.

Sign up for GitHub

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

Jump to bottom

allow string literals for enums! #2484

Merged
merged 14 commits into from
May 21, 2018
Merged

allow string literals for enums! #2484

merged 14 commits into from
May 21, 2018

Conversation

giladgray
Copy link
Contributor

@giladgray giladgray commented May 10, 2018 •
edited
Loading

Fixes #2476

convert many former string enums to const/type pairs so you can do <Button intent="primary" /> 👍

the new convention is as follows:

  • enum types should always use numbers, unless there's a very good reason for string types
  • if it's a string enum, it should probably be a const/type pair
  • if it's used in a props interface, it should be a const/type pair

converted enums:

  • Alignment
  • Intent
  • Position
  • Elevation (and break circular dep)
  • PopoverInteractionKind
  • 🔥 TimePickerPrecision ⇒ TimePrecision
  • 🔥 CollapseFrom ⇒ Boundary
  • 🔥 DateRangeBoundary ⇒ Boundary (from core)

⚠️ blocked on #2510 for docs support

@blueprint-bot
Copy link

fix timepicker tests

Preview: documentation | landing | table

@blueprint-bot
Copy link

fix DateInputTests

Preview: documentation | landing | table

@giladgray giladgray mentioned this pull request May 16, 2018
Merged
@giladgray giladgray requested a review from invliD May 16, 2018 20:51
@blueprint-bot
Copy link

Merge branch 'develop' of github.com:palantir/blueprint into gg/string-literals

Preview: documentation | landing | table

invliD
invliD approved these changes May 16, 2018
View reviewed changes
packages/test-commons/src/utils.ts
FIREFOX,
IE,
UNKNOWN,
WEBKIT,
Copy link
Member

Choose a reason for hiding this comment

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

why an integer enum?

Copy link
Contributor Author

@giladgray giladgray May 16, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

it's entirely internal to this file (not exported) and i declared in the description that actual enum types should prefer numbers unless they have a good reason for strings.

@giladgray
Copy link
Contributor Author

unblocked! functional docs

@blueprint-bot
Copy link

Merge branch 'develop' of github.com:palantir/blueprint into gg/string-literals

Preview: documentation | landing | table

@giladgray giladgray merged commit c7986db into develop May 21, 2018
@giladgray giladgray deleted the gg/string-literals branch May 21, 2018 23:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@invliD invliD invliD approved these changes

@llorca llorca Awaiting requested review from llorca

@themadcreator themadcreator Awaiting requested review from themadcreator

@adidahiya adidahiya Awaiting requested review from adidahiya

Assignees
No one assigned
Labels
Type: breaking change
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@giladgray @blueprint-bot @invliD